On 2025/07/02 23:31, Fujii Masao wrote:
On 2025/07/01 3:27, Álvaro Herrera wrote:
On 2025-Jun-30, Álvaro Herrera wrote:
Just one note: Jian's patch doesn't handle the same issue for TRIGGER
case, so that part might still need to be addressed.
Okay, here's my take on this, wherein I reworded the proposed error
message. I also handled the NOT VALID case of a constraint trigger; maybe my
patch is too focused on that specific bit and instead we should handle
also NO INHERIT and NOT ENFORCED cases, not really sure (it's certainly
not an important part of this patch).
For ease of review, here's the three patches. 0001 solves the main
problem with ALTER objtype ALTER CONSTRAINT NOT VALID.
Thanks for updating the patches! Patch 0001 looks good to me.
I have one question though: why didn't you include an error code in
the error message? I was thinking it would be fine to use
errcode(ERRCODE_FEATURE_NOT_SUPPORTED), like other error messages
in processCASbits(), since ALTER CONSTRAINT NOT VALID isn't supported.
I propose to put 0001 in 18 and 19, and leave 0002 and 0003 (as a single
commit) for 19 only, since it's been like that for ages and there have
been zero complaints before my own in the other thread.
Agreed.
I put 0002 as a
separate one just for review, to show that these errors we throw are
nothing new: these commands would also fail if we don't patch this code,
they're just using bad grammar, which is then fixed by 0003.
Regarding the 0003 patch:
+ if (($11 & CAS_NOT_ENFORCED) != 0)
+ ereport(ERROR,
+ errmsg("constraint
triggers cannot be marked %s",
+ "NOT
ENFORCED"),
+
parser_errposition(@11));
Shouldn't we also raise an error when CAS_ENFORCED is given?
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
