On 2017/01/05 8:05, Tom Lane wrote:
> Ashutosh Bapat <ashutosh.bapat@enterprisedb.com> writes:
>> Right. But I think it's better to use attribute id, in case the code
>> raising this error changes for any reason in future.
>
> I agree. The parent's "tdhasoid" flag is definitely based on the
> existence of an ObjectIdAttributeNumber system column, not on whether the
> column's name is "oid". So doing a lookup by name to find the matching
> child column is just weird, and cannot possibly lead to anything good.
You beat me to revising the patch along the lines suggested by Ashutosh.
>> The code updating attinhcount and then updating the catalogs is same
>> for user defined attributes and OID. Should we separate it out into a
>> function and use that function instead of duplicating the code?
>
> Didn't really seem worth the trouble ... maybe if it gets any longer
> it'd be appropriate to do that.
>
>> Your test uses tablenames starting with "_". I have not seen that
>> style in the testcases. Is it intentional?
>
> Yeah, I did not like that either.
>
> Pushed with those corrections and some further fooling with the test case.
Thanks for reviewing and committing the patch!
Regards,
Amit