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. 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.
>
>
> I think I should list Amul as the fourth co-author of 0001. That would
> make the longest list of coauthorship for a patch that small. Or I
> could just say: "Author: Postgres Global Development Group".
If the patch is based on Amul's work, I agree it makes sense to add
him as a co-author in the commit log.
Regards,
--
Fujii Masao
NTT DATA Japan Corporation
