Обсуждение: Re: refactor AlterDomainAddConstraint (alter domain add constraint)
Hello, On 2025-Jan-15, jian he wrote: > we cannot error out AlterDomainAddConstraint for cases like ALTER > DOMAIN ADD CHECK NO INHERIT. > because "NO INHERIT" is actually a separate Constraint Node, and > AlterDomainAddConstraint > can only handle one Constraint node. I had forgotten this thread, and I ended up implementing a different solution for this issue, which I just posted at https://postgr.es/m/202503101758.ipn3g64twfye@alvherre.pgsql I like my patch better than this approach because it allows us to solve the same problem as it appears in other parts of the grammar, and also because it avoids the bit fiddling which is harder to maintain later on. If you'd care to have a look at it, I'd appreciate it. Thanks -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
On 10.03.25 19:37, Alvaro Herrera wrote: > On 2025-Jan-15, jian he wrote: >> we cannot error out AlterDomainAddConstraint for cases like ALTER >> DOMAIN ADD CHECK NO INHERIT. >> because "NO INHERIT" is actually a separate Constraint Node, and >> AlterDomainAddConstraint >> can only handle one Constraint node. > > I had forgotten this thread, and I ended up implementing a different > solution for this issue, which I just posted at > https://postgr.es/m/202503101758.ipn3g64twfye@alvherre.pgsql > > I like my patch better than this approach because it allows us to solve > the same problem as it appears in other parts of the grammar, and also > because it avoids the bit fiddling which is harder to maintain later on. > If you'd care to have a look at it, I'd appreciate it. Where are we on this? Which of the two patches should we pursue?
On Mon, Nov 24, 2025 at 10:45 PM Peter Eisentraut <peter@eisentraut.org> wrote: > > On 10.03.25 19:37, Alvaro Herrera wrote: > > > > I had forgotten this thread, and I ended up implementing a different > > solution for this issue, which I just posted at > > https://postgr.es/m/202503101758.ipn3g64twfye@alvherre.pgsql > > > > I like my patch better than this approach because it allows us to solve > > the same problem as it appears in other parts of the grammar, and also > > because it avoids the bit fiddling which is harder to maintain later on. > > If you'd care to have a look at it, I'd appreciate it. > > Where are we on this? Which of the two patches should we pursue? > hi. if you go to this link https://postgr.es/m/202503101758.ipn3g64twfye@alvherre.pgsql check v2-0001-Improve-processCASbits-API-with-a-seen-struct.patch you will find that it added a struct CAS_flags to processCASbits. +typedef struct CAS_flags +{ + bool seen_deferrability; + bool seen_enforced; + bool seen_valid; + bool seen_inherit; +} CAS_flags; In v2-0001, most of the case processCASbits just pass a NULL CAS_flags(seen). CAS_flags are not used in table constraints at all. but CAS_flags indeed used for error message handling in ALTER DOMAIN ADD CONSTRAINT. (IMHO, it looks like big efforts to solve the same problem, also these bit fiddling for domain constraint unlikely to change in the future, e.g. we are unlike to add DEFERRABLE, INITIALLY DEFERRED, NO INHERIT constraints to domain.) anyway, both patches are attached. this thread: v5-0001-ALTER-DOMAIN-ADD-CONSTRAINT-error-message-enhance.patch thread: https://postgr.es/m/202503101758.ipn3g64twfye@alvherre.pgsql v5-0001-Improve-processCASbits-API-with-a-seen-struct.no-cfbot v5-0002-handle-constraints-on-domains-too.no-cfbot -- jian https://www.enterprisedb.com
Вложения
On 2025-Nov-26, jian he wrote: > (IMHO, it looks like big efforts to solve the same problem, also these bit > fiddling for domain constraint unlikely to change in the future, e.g. we are > unlike to add DEFERRABLE, INITIALLY DEFERRED, NO INHERIT constraints to domain.) True. I was thinking we would use the this mechanism to solve the issue for constraint triggers, but in commit 87251e114967 I did that in a different way. I think this one > this thread: v5-0001-ALTER-DOMAIN-ADD-CONSTRAINT-error-message-enhance.patch is a reasonable implementation, ugly though it is. We could handle the checks for deferrability before calling processCASbits() though. -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ Tom: There seems to be something broken here. Teodor: I'm in sackcloth and ashes... Fixed. http://postgr.es/m/482D1632.8010507@sigaev.ru