Re: CREATE LIKE INCLUDING COMMENTS and STORAGES

2009/9/7 Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)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