Re: Review: Non-inheritable check constraints - Mailing list pgsql-hackers

From Nikhil Sontakke
Subject Re: Review: Non-inheritable check constraints
Date
Msg-id CANgU5ZcfEJhjYT742wQq72ntaST5-djtj9ExiDVZ7R2f=tQj_w@mail.gmail.com
Whole thread Raw
In response to Review: Non-inheritable check constraints  (Alex Hunsaker <badalex@gmail.com>)
Responses Re: Review: Non-inheritable check constraints
List pgsql-hackers
Hi Alex,

I didn't care for the changes to gram.y so I reworked it a bit so we
now pass is_only to AddRelationNewConstraint() (like we do with
is_local). Seemed simpler but maybe I missed something. Comments?


Hmmm, your patch checks for a constraint being "only" via:

              !recurse && !recursing

I hope that is good enough to conclusively conclude that the constraint is 'only'. This check was not too readable in the existing code for me anyways ;). If we check at the grammar level, we can be sure. But I remember not being too comfortable about the right position to ascertain this characteristic.
 
I also moved the is_only check in AtAddCheckConstraint() to before we
grab and loop through any children. Seemed a bit wasteful to loop
through the new constraints just to set a flag so that we could bail
out while looping through the children.


Ditto comment for this function. I thought this function went to great lengths to spit out a proper error in case of inconsistencies between parent and child. But if your check makes it simpler, that's good!
 
You also forgot to bump Natts_pg_constraint.


Ouch. Thanks for the catch.
 
PFA the above changes as well as being "rebased" against master.

Thanks Alex, appreciate the review!

Regards,
Nikhils

pgsql-hackers by date:

Previous
From: Alexander Korotkov
Date:
Subject: Re: Double sorting split patch
Next
From: Oleg Bartunov
Date:
Subject: Re: WIP: SP-GiST, Space-Partitioned GiST