Re: ALTER TABLE ALTER CONSTRAINT misleading error message

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