Feedback-ID: ia2694551:Fastmail
Date: Fri, 21 Mar 2025 19:04:10 +0100
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Rushabh Lathia <rushabh.lathia@gmail.com>,
	Ashutosh Bapat <ashutosh.bapat.oss@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Support NOT VALID / VALIDATE constraint options for named NOT
 NULL constraints
Message-ID: <202503211804.xlgvkeq4dhc4@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CA+Tgmob3J7KjCTNJR5v-jqg3C-7TYmsB239sUUHr0+DOtie4vg@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202503211804.xlgvkeq4dhc4%40alvherre.pgsql>
Precedence: bulk

On 2025-Mar-21, Robert Haas wrote:

> I don't agree with this conclusion.

Uhm.

> The 8.3 casting changes were problematic because any piece of SQL
> you'd ever written could have problems.

Okay, this much I agree with.

> This change will only break queries that look at the attnotnull
> column.  While there may be quite a few of those, it can't possibly be
> of the same order. I think it's routine that changing the name or type
> of system catalog columns breaks things for a few people (e.g.
> procpid->pid, or relistemp->relpersistence) and we sometimes get
> complaints about that, but at least you can grep for it and it's
> mostly going to affect admin tools rather than all the queries
> everywhere.

In several of the cases that I checked, the application just tests the
returned value for boolean truth.  If we change the column from boolean
to char, they would stop working properly because both the 't' and the
'f' char values would test as true.  But suppose we were to rename the
column; that would cause developers to have to examine the code to
determine how to react.  That might even be good, because we're end up
in a situation were no application uses outdated assumptions about
nullness in a column.  However, consider the rationale given in 
https://postgr.es/m/2542644.1733418030@sss.pgh.pa.us
that removing attndims would break PHP -- after that discussion, we
decided against removing the column, even though it's completely
useless, because we don't want to break PHP.  You know, removing
attnotnull would break PHP in exactly the same way, or maybe in some
worse way.  I don't see how can we reach a different conclusion for this
change that for that one.

> That's not to say that adding a second bool column instead of changing
> the existing column's data type is necessarily the wrong way to go.
> But I think you're overestimating the blast radius by quite a lot.

I am just going by some truth established by previous discussion.
If we agree to remove attnotnull or to change the way it works, then we
can also remove attndims.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/