2009/9/7 Itagaki Takahiro <itagaki.takah...@oss.ntt.co.jp>: > Here is a patch to implement the following items in our ToDo list: > * Add CREATE TABLE LIKE ... INCLUDING COMMENTS > * Have CREATE TABLE LIKE copy column storage parameters >
Hello Itagaki-san, I am doing an initial review of your patch. I applied the version labelled 20090908 (applied with minor fuzz to HEAD). It compiled cleanly and the feature appears to work as advertised. I did a little bit of copy-editing on my way through (changes attached) but the patch seems to be in very good shape. The documentation is clearly worded, although I did add a cross-reference in the bit about STORAGE. The regression tests seem to give a pretty good coverage of both the success and failure modes. In response to the questions you raised in your post: > * Should INCLUDING COMMENTS also copy comments on indexes? > It copies only comments on columns for now. It probably should, but if this is difficult to work in, I don't see anything wrong with leaving it out of this patch and making it a TODO. > > * Should we have additonal syntax to define storage parameters inline > of CREATE TABLE? For example, > CREATE TABLE tbl (col text STORAGE MAIN); > CREATE TABLE fails if there is a conflicted storage parameter for now. > ERROR: column "col" has a storage parameter conflict > DETAIL: MAIN versus EXTENDED > but there is no way to resolve the confliction unless we modify the > definitions of original tables. Meantime, we can overwrite DEFAULTs > to resolve conflictions by INCLUDING DEFAULTS. I think I'm failing to understand why this would be an issue. Why would the user be specifying columns in the CREATE TABLE statement that already exist in the table they are cloning? > > * Should we have "INCLUDING ALL" as an abbreviated form? > Many INCLUDING options in CREATE LIKE seems to be messy: > CREATE TABLE clone_table (LIKE template_table > INCLUDING DEFAULTS > INCLUDING CONSTRAINTS > INCLUDING INDEXES > INCLUDING STORAGES > INCLUDING COMMENTS); > +1 for adding INCLUDING ALL. The grammar should also support EXCLUDING ALL for symmetry, even though EXCLUDING ALL is the default behaviour. However I do think that this should be a separate patch ... add to TODO? Cheers, BJ
create-including-20090908-changes.diff
Description: Binary data
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
