|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>|
|Subject:||Re: Copy column storage parameters on CREATE TABLE LIKE/INHERITS|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
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
|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|