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