Re: bogus error message for ALTER TABLE ALTER CONSTRAINT

On 2025-Mar-04, Tom Lane wrote:

> Hmm. I agree that "ALTER CONSTRAINT statement" is off the
> mark here, but I'm not convinced that "FOREIGN KEY" is entirely
> on-point either. The grammar has no way of knowing what kind of
> constraint is being targeted. I do see that ATExecAlterConstraint
> currently rejects every other kind of constraint, but do we need
> to think of a more generic phrase?

You're right that this is bogus, and looking what to do about it made me
realize that CREATE CONSTRAINT TRIGGER is also saying bogus things such
as:

create constraint trigger foo after insert on pg_class not valid for each row execute procedure test();
ERROR: TRIGGER constraints cannot be marked NOT VALID

create constraint trigger foo after insert on pg_class no inherit for each row execute procedure test();
ERROR: TRIGGER constraints cannot be marked NO INHERIT

So after mulling over this for a while I came up with the idea of adding
an output struct that processCASbits() can optionally be given and fill,
which indicates which flags were seeing while parsing the options. With
that, each of these two callers can throw more appropriate error messages
when a flag is given that they don't like. This is much better,
though arguably the error messages I propose can be wordsmithed still.
Most callers of processCASbits() don't care, so they just give a NULL
and they get the current behavior.

In the current incantation I just pass a "bool dummy" for the bits that
each production doesn't support; processCASbits throws no error and
instead sets the corresponding flag in the 'seen' struct. However,
another option might be to not pass a dummy at all and just
conditionally not throw any errors when the 'seen' struct is given.
This might be cleaner, but it also feels a bit magical. Any
preferences?

By the way, this also made me realize that the addition of a SET keyword
in the commands
ALTER TABLE .. ALTER CONSTRAINT SET NO INHERIT
ALTER TABLE .. ALTER CONSTRAINT SET INHERIT
could be removed easily by taking advantage of this 'seen' struct, and
we'd remove one production from the grammar (or both new ones, if we add
INHERIT to ConstraintAttributeElem).

--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
"Debido a que la velocidad de la luz es mucho mayor que la del sonido,
algunas personas nos parecen brillantes un minuto antes
de escuchar las pelotudeces que dicen." (Roberto Fontanarrosa)