From: | jian he <jian(dot)universality(at)gmail(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> |
Cc: | Álvaro Herrera <alvherre(at)kurilemu(dot)de>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: ALTER TABLE ALTER CONSTRAINT misleading error message |
Date: | 2025-06-13 07:10:50 |
Message-ID: | CACJufxEboWWMjWSkQPrdP0RAxg9vH49vcuB=U0gud2pbF2rP=Q@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Wed, Jun 11, 2025 at 10:20 PM Fujii Masao
<masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>
> >> 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.
>
errmsg("cannot alter constraint validity")
can mean
ALTER TABLE ... ALTER CONSTRAINT ... VALID
ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID
even though ALTER TABLE ... ALTER CONSTRAINT ... VALID would yield
syntax errors.
message saying ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID
would make it more explicit.
Hence changed to:
errmsg("ALTER TABLE ... ALTER CONSTRAINT ... NOT VALID is not supported")
Attachment | Content-Type | Size |
---|---|---|
v2-0001-error-handling-for-ALTER-CONSTRAINT-NOT-VALID.no-cfbot | application/octet-stream | 2.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2025-06-13 07:19:26 | Allow pg_dump --statistics-only to dump foreign table statistics? |
Previous Message | shveta malik | 2025-06-13 06:52:10 | Re: Conflict detection for update_deleted in logical replication |