Re: Review: Non-inheritable check constraints
| От | Nikhil Sontakke |
|---|---|
| Тема | Re: Review: Non-inheritable check constraints |
| Дата | |
| Msg-id | CANgU5ZcfEJhjYT742wQq72ntaST5-djtj9ExiDVZ7R2f=tQj_w@mail.gmail.com обсуждение исходный текст |
| Ответ на | Review: Non-inheritable check constraints (Alex Hunsaker <badalex@gmail.com>) |
| Ответы |
Re: Review: Non-inheritable check constraints
|
| Список | pgsql-hackers |
Hi Alex,
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.
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!
Ouch. Thanks for the catch.
Thanks Alex, appreciate the review!
Regards,
Nikhils
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 по дате отправления: