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
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 |