Thread: Copy column storage parameters on CREATE TABLE LIKE/INHERITS
Here is an updated version of the "Copy storage parameters" patch. http://archives.postgresql.org/pgsql-hackers/2008-07/msg01417.php This patch copies only column storage parameters and does not copy reloptions as a result of the discussion, in which reloptions should not be copied because LIKE and INHERITS are not table definitions, but column definitions. Copying reloptions (or copying just table's definition) might be useful in some cases, but it should be done by another independent patch. Inheriting column storage parameters is useful if we duplicate an existing table. CREATE TABLE AS is useful to avoid WALs in that time: CREATE TABLE (LIKE target) WITH (<same as the target table>) AS SELECT * FROM target; However, we cannot execute ALTER COLUMN SET STORAGE after creating the new table and before inserting, because there are no time to execute commands between them. So we need some methods to set column storage parameters at creating a table. Regards, --- ITAGAKI Takahiro NTT Open Source Software Center
Attachment
[Patch Review] Copy column storage parameters on CREATE TABLE LIKE/INHERITS
From
Stephen Frost
Date:
Greetings, I've reviewed the patch here: http://archives.postgresql.org/message-id/20080812170932.8E65.52131E4D@oss.ntt.co.jp and am happy to report that it looks to be in good order. It has documentation and regression updates, is in context diffformat, patches against current CVS with only some minor fuzz, and appears to work as advertised. I looked over thepatch and could easily follow what was going on, did some additional testing beyond the regression tests, and reviewedthe documentation changes. My only comment on this patch is that the documentation updates might include a link back to Section 53.2 for more information,and/or to the SET STORAGE portion of the alter table/alter column command documentation, since the only other'storage' reference in create table's documentation is about table storage parameters. Thanks! Stephen
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