From: | Nikhil Sontakke <nikkhils(at)gmail(dot)com> |
---|---|
To: | Alex Hunsaker <badalex(at)gmail(dot)com> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Review: Non-inheritable check constraints |
Date: | 2011-10-06 08:42:38 |
Message-ID: | CANgU5ZcfEJhjYT742wQq72ntaST5-djtj9ExiDVZ7R2f=tQj_w@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Oleg Bartunov | 2011-10-06 08:58:59 | Re: WIP: SP-GiST, Space-Partitioned GiST |
Previous Message | Alexander Korotkov | 2011-10-06 08:22:19 | Re: Double sorting split patch |