>>
>> We allow creating user attribute with name "oid" so you do not want to
>> search system attribute oid by name. Instead search by attribute id
>> ObjectIdAttributeNumber.
>
> Good point. Although, there can only be one of the two in a table at any
> given time - either the "oid" system column or user-defined column named
> "oid". I was afraid that with the patch, the attinhcount of a
> user-defined oid column may get incremented twice, but before that could
> happen, one would get the error: "child table without OIDs cannot
> inherited from table with OIDs"
>
> create table foo (a int) with oids;
> create table bar (a int, oid int);
> alter table bar inherit foo;
> ERROR: table "bar" without OIDs cannot inherit from table "foo" with OIDs
>
> And then:
>
> alter table bar set with oids;
> ERROR: column "oid" of relation "bar" already exists
>
Right. But I think it's better to use attribute id, in case the code
raising this error changes for any reason in future. Or at least we
should add an assert or an elog to make sure that we are updating the
system attribute OID and not user defined one.
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? The
problem with duplicating the code is one has to remember to update
both copies while changing it.
Your test uses tablenames starting with "_". I have not seen that
style in the testcases. Is it intentional?
Rest of the patch looks good to me.
--
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company
