Re: ALTER TABLE parent SET WITHOUT OIDS and the oid column

From: Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)com>
Cc: Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: ALTER TABLE parent SET WITHOUT OIDS and the oid column
Date: 2017-01-05 04:33:00
Message-ID: 1238eb68-b909-571d-016f-9b2a01051c12@lab.ntt.co.jp
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2017/01/05 8:05, Tom Lane wrote:
> Ashutosh Bapat <ashutosh(dot)bapat(at)enterprisedb(dot)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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2017-01-05 04:50:11 Re: [HACKERS] Questionable tag usage
Previous Message Ashutosh Bapat 2017-01-05 04:30:42 Re: Reporting planning time with EXPLAIN