On 2025/06/27 22:30, Álvaro Herrera wrote:
On 2025-06-27, Fujii Masao wrote:
To make this distinction, I just started thinking it's better to raise
the error
in ATExecAlterConstraint() rather than in gram.y. I've attached a draft
patch that
follows this approach.
Hmm I don't like this very much, it feels very kludgy. I think if we want to
consider this in scope for pg18 I would much prefer to use the patch I
mentioned near the beginning of the thread.
Are you referring to v2-0001-trial.patch proposed at [1]?
Regarding this patch:
- c->alterDeferrability = true;
- processCASbits($4, @4, "FOREIGN KEY",
+ processCASbits($4, @4, NULL,
&c->deferrable,
&c->initdeferred,
NULL,
NULL, NULL, yyscanner);
+ c->alterDeferrability =
ALTER_DEFERRABILITY($4);
+ /* cannot (currently) be changed by
this syntax: */
+ if (ALTER_ENFORCEABILITY($4))
+ ereport(ERROR,
+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter
constraint enforceability"),
+
parser_errposition(@4));
+ if (ALTER_VALID($4))
+ ereport(ERROR,
+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter
constraint validity"),
+
parser_errposition(@4));
+ if (ALTER_INHERIT($4))
+ ereport(ERROR,
+
errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot alter
constraint inheritability"),
+
parser_errposition(@4));
This patch raises an error if ENFORCED, NOT ENFORCED, INHERIT, or NO INHERIT
is specified. But those options are currently accepted, so these checks seem
unnecessary for now.
Also, the patch raises an error for NOT VALID after calling processCASbits(),
there's no need to call processCASbits() in the first place. It would be
cleaner to check for NOT VALID and raise an error before calling it.
Jian He already proposed this approach in a patch at [2].
{
if (deferrable)
*deferrable = true;
- else
+ else if (constrType)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
/* translator: %s is CHECK, UNIQUE, or similar */
If the NOT VALID check is moved before processCASbits(), the above changes
inside processCASbits() also become unnecessary. Jian's patch doesn't change
processCASbits() this way.
It looks like Jian's patch at [2] is an updated version of the one you referred
to,
so you may agree with that approach. Thought?
Just one note: Jian's patch doesn't handle the same issue for TRIGGER case,
so that part might still need to be addressed.
Regards,
[1]
https://www.postgresql.org/message-id/caaj_b97hd-jmts7ajgu6tdbczdx_kyukxg+k-dtymoieg+g...@mail.gmail.com
[2]
https://postgr.es/m/CACJufxH9krMV-rJkC1J=jvy_fao_nrvxgmv+dsnm2sahjbu...@mail.gmail.com
--
Fujii Masao
NTT DATA Japan Corporation
