Re: ALTER TABLE ALTER CONSTRAINT misleading error message

On 2025/06/02 12:13, jian he wrote:
> On Wed, May 28, 2025 at 7:59 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
>>
>> On 2025-May-28, jian he wrote:
>>
>>> hi.
>>>
>>> create table t(a int, constraint cc check(a  = 1));
>>> ALTER TABLE t ALTER CONSTRAINT cc not valid;
>>> ERROR:  FOREIGN KEY constraints cannot be marked NOT VALID
>>> LINE 1: ALTER TABLE t ALTER CONSTRAINT cc not valid;
>>>                                            ^
>>>
>>> the error message seems misleading,

I also ran into this issue while testing constraints with NOT VALID.


>> We discussed this already, didn't we?  There's a thread with IIRC three
>> proposed patches for this.  I think I liked this one the most:
>>
>> https://postgr.es/m/CAAJ_b97hd-jMTS7AjgU6TDBCzDx_KyuKxG+K-DtYmOieg+giyQ@mail.gmail.com
>>
>
> for ALTER CONSTRAINT,
> we already handled most error cases in ATExecAlterConstraint.
>
>      if (cmdcon->alterDeferrability && currcon->contype != CONSTRAINT_FOREIGN)
>          ereport(ERROR,
>                  (errcode(ERRCODE_WRONG_OBJECT_TYPE),
>                   errmsg("constraint \"%s\" of relation \"%s\" is not a
> foreign key constraint",
>                          cmdcon->conname, RelationGetRelationName(rel))));
>      if (cmdcon->alterEnforceability && currcon->contype != CONSTRAINT_FOREIGN)
>          ereport(ERROR,
>                  (errcode(ERRCODE_WRONG_OBJECT_TYPE),
>                   errmsg("cannot alter enforceability of constraint
> \"%s\" of relation \"%s\"",
>                          cmdcon->conname, RelationGetRelationName(rel))));
>      if (cmdcon->alterInheritability &&
>          currcon->contype != CONSTRAINT_NOTNULL)
>          ereport(ERROR,
>                  errcode(ERRCODE_WRONG_OBJECT_TYPE),
>                  errmsg("constraint \"%s\" of relation \"%s\" is not a
> not-null constraint",
>                         cmdcon->conname, RelationGetRelationName(rel)));
>
> but ATExecAlterConstraint didn't handle  "ALTER CONSTRAINT NOT VALID",
> it was handled in processCASbits.
>
> so the attached minimum patch (extract from v2-0001-trial.patch)
> is fine for PG18, IMHO.

+                        ereport(ERROR,
+                                errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                errmsg("cannot alter constraint validity"),

Since ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID isn't supported,
how about making the error message more specific? For example:

     "ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID is not supported"

This would make it clearer to users what exactly isn't allowed.

Regards,

--
Fujii Masao
NTT DATA Japan Corporation