Feedback-ID: ia2694551:Fastmail
Date: Tue, 11 Mar 2025 11:21:12 +0100
From: =?utf-8?Q?=C3=81lvaro?= Herrera <alvherre@alvh.no-ip.org>
To: jian he <jian.universality@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
	Pg Hackers <pgsql-hackers@lists.postgresql.org>,
 Peter Eisentraut <peter@eisentraut.org>,
	Nathan Bossart <nathandbossart@gmail.com>
Subject: Re: bogus error message for ALTER TABLE ALTER CONSTRAINT
Message-ID: <202503111021.xz4itftejc2f@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CACJufxHrbMph6QQaC8sxBencsRJ9ajZ2LL76Rv5BN2qAVRjX8g@mail.gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/202503111021.xz4itftejc2f%40alvherre.pgsql>
Precedence: bulk

On 2025-Mar-11, jian he wrote:

> this look a little strange?
>     if (cas_bits & (CAS_NOT_DEFERRABLE) && seen)
>         seen->seen_deferrability = true;
> 
> it should be
>     if ((cas_bits & CAS_NOT_DEFERRABLE) && seen)
>         seen->seen_deferrability = true;

True.  And since you mentioned CAS_INITIALLY_IMMEDIATE, it should really
be

	/* These are the default values; just report that we saw them */
	if ((cas_bits & (CAS_NOT_DEFERRABLE | CAS_INITIALLY_IMMEDIATE)) && seen)
		seen->seen_deferrability = true;


> typedef struct CAS_flags need add to typedefs.list

True.

> seems didn't cover "initially immediate" case for domain.
> for example:
> create domain d_int as int4;
> --- the following two cases should fail.
> alter domain d_int add constraint nn1 not null initially immediate;
> alter domain d_int add constraint cc check(value > 1) initially immediate;

Yeah, I thought at first you were right, but on further thought, do we
really want to do this?  I mean, INITIALLY IMMEDIATE is the default
timing for a constraint, so why should we complain if a user wants to
get a constraint that's declared that way?  I'm not sure that we should
do it.  Same with NOT DEFERRABLE.
[... looks at the standard doc ...]
And that's indeed what the SQL standard says:

<domain definition> ::=
  CREATE DOMAIN <domain name> [ AS ] <predefined type>
      [ <default clause> ]
      [ <domain constraint>... ]
      [ <collate clause> ]

<domain constraint> ::=
  [ <constraint name definition> ] <check constraint definition> [ <constraint characteristics> ]

8) For every <domain constraint> specified:
   [...]
b) If <constraint characteristics> is not specified, then INITIALLY IMMEDIATE
   NOT DEFERRABLE is implicit.


So I think the fix here needs to distinguish those cases and avoid
throwing errors for them.

(Note also it does not say that INITIALLY DEFERRED or DEFERRABLE are
disallowed, which means that we're failing to fully implement the
standard-mandated behavior by prohibiting those.)


> create domain d_int as int4;
> alter domain d_int add not null no inherit not valid;
> ERROR:  not-null constraints on domains cannot be marked NOT VALID
> LINE 1: alter domain d_int add not null no inherit not valid;
>                                         ^
> If we can report an error like
> "ERROR:  NOT NULL constraints on domains cannot be marked INHERIT / NOT INHERIT"
> That would be great.
> just report the first constraint property that is not ok, but seems not doable.

Yeah, I don't think this can be made to work.  Maybe we'd have to change
the way ConstraintAttributeSpec is parsed completely.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Once again, thank you and all of the developers for your hard work on
PostgreSQL.  This is by far the most pleasant management experience of
any database I've worked on."                             (Dan Harris)
http://archives.postgresql.org/pgsql-performance/2006-04/msg00247.php