Re: Copy column storage parameters on CREATE TABLE LIKE/INHERITS

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: ITAGAKI Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp>
Cc: HACKERS <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Copy column storage parameters on CREATE TABLE LIKE/INHERITS
Date: 2008-09-11 20:33:26
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

ITAGAKI Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> writes:
> Here is an updated version of the "Copy storage parameters" patch.

I looked this over and feel that it still needs work. The
implementation seems appropriate for columns coming from LIKE clauses,
but there are two big issues for columns coming from inheritance parent

* The patch opens and acquires lock on the parent relations far too
early. That is supposed to happen in DefineRelation, which among other
things contains the appropriate permissions check. It would be possible
to use this patch to hold AccessShareLock for a long time on tables you
have no permissions for.

* There is no consideration for merging of similarly-named columns
coming from different parent tables. The general approach taken in
DefineRelation is to throw an error if there are incompatible properties
in columns to be merged, and I think the same should happen for storage
properties. What you actually would get, here, is that some random one
of the columns would "win".

In short, I think you need two implementations, one for LIKE and one
for INHERITS, and the appropriate place to tackle the latter case is in
DefineRelation (or even more specifically, MergeAttributes).

Also I concur with Stephen Frost's suggestion to add a couple of
cross-references to the documentation patches.

regards, tom lane

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Brendan Jurd 2008-09-11 20:53:55 Re: Parsing of pg_hba.conf and authentication inconsistencies
Previous Message Andrew Chernow 2008-09-11 20:08:48 Re: Commitfest patches mostly assigned ... status