Re: Copy column storage parameters on CREATE TABLE LIKE/INHERITS - Mailing list pgsql-hackers

From Tom Lane
Subject Re: Copy column storage parameters on CREATE TABLE LIKE/INHERITS
Date
Msg-id 13146.1221165206@sss.pgh.pa.us
Whole thread Raw
In response to Copy column storage parameters on CREATE TABLE LIKE/INHERITS  (ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>)
List pgsql-hackers
ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp> writes:
> Here is an updated version of the "Copy storage parameters" patch.
> http://archives.postgresql.org/pgsql-hackers/2008-07/msg01417.php

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

* 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


pgsql-hackers by date:

Previous
From: Andrew Chernow
Date:
Subject: Re: Commitfest patches mostly assigned ... status
Next
From: "Brendan Jurd"
Date:
Subject: Re: Parsing of pg_hba.conf and authentication inconsistencies