add additional options to CREATE TABLE ... AS

This patch adds most of the options available for regular CREATE TABLE
syntax to the CREATE TABLE x AS SELECT ... and AS EXECUTE ...
Specifically this allows specification of on commit behavior for temp
tables and tablespaces for regular tables to these two statements.
Additionally with/without oids is now available for the EXECUTE variant.
Currently you still cannot specify inheritance attributes with these
commands, but this seems like a more complicated task.

Kris Jurka

On Tue, 2006-02-14 at 14:32 -0500, Kris Jurka wrote:
> This patch adds most of the options available for regular CREATE TABLE
> syntax to the CREATE TABLE x AS SELECT ... and AS EXECUTE ...
> Specifically this allows specification of on commit behavior for temp
> tables and tablespaces for regular tables to these two statements.

The implementation is pretty ugly -- it clutters ExecuteStmt and Query
with fields that really do not belong there. Per previous discussion, I
think it would be better to refactor the CREATE TABLE AS implementation
to be essentially a CREATE TABLE followed by a INSERT ... SELECT.

(That's not necessarily a reason to reject the patch, but the patch does
increase the benefit of performing that refactoring.)

A few cosmetic comments:

typedef struct ExecuteStmt
{
NodeTag type;
char *name;
RangeVar *into;
ContainsOids intocontainsoids;
bool intohasoids;
OnCommitAction intooncommit;
char *intotablespacename;
List *params;
} ExecuteStmt;

I think we ought to use either camel-case or underscore characters to
separate words.

parser/analyze.c, circa 1822:

if (stmt->intoTableSpaceName)
qry->intoTableSpaceName = pstrdup(stmt->intoTableSpaceName);
else
qry->intoTableSpaceName = NULL;

You can omit the "else", as makeNode() zeroes all the fields of the new
node. (You could argue that leaving the assignment is more readable, but
I personally don't think so: this behavior of makeNode() is used in a
several places in the backend.)

-Neil

Neil Conway <neilc(at)samurai(dot)com> writes:
> The implementation is pretty ugly -- it clutters ExecuteStmt and Query
> with fields that really do not belong there. Per previous discussion, I
> think it would be better to refactor the CREATE TABLE AS implementation
> to be essentially a CREATE TABLE followed by a INSERT ... SELECT.

I kinda wonder why bother at all. I don't see any good reason why
people shouldn't issue two statements.

>> if (stmt->intoTableSpaceName)
>> qry->intoTableSpaceName = pstrdup(stmt->intoTableSpaceName);
>> else
>> qry->intoTableSpaceName = NULL;

> You can omit the "else", as makeNode() zeroes all the fields of the new
> node.

For that matter, why not just

qry->intoTableSpaceName = stmt->intoTableSpaceName;

There's no need for the string-copy operation here, is there?

regards, tom lane

On Tue, 14 Feb 2006, Kris Jurka wrote:

>
> This patch adds most of the options available for regular CREATE TABLE syntax
> to the CREATE TABLE x AS SELECT ... and AS EXECUTE ...

Here's the doc changes for this.

Kris Jurka

On Tue, 14 Feb 2006, Tom Lane wrote:

> I kinda wonder why bother at all. I don't see any good reason why
> people shouldn't issue two statements.
>

Well if you don't know what the resulting columns are going to be that
could be difficult. There are a number of reasons why this patch is an
improvement.

1) People have requested this feature:
http://archives.postgresql.org/pgsql-bugs/2005-11/msg00163.php
http://archives.postgresql.org/pgsql-bugs/2004-03/msg00186.php

2) The SQL spec requires ON COMMIT for CREATE TEMP TABLE AS SELECT.

3) The unification of EXECUTE and SELECT options actually simplifies the
grammar by removing the WithOidsAs production hack.

Kris Jurka

On Tue, 2006-02-14 at 15:38 -0500, Tom Lane wrote:
> Neil Conway <neilc(at)samurai(dot)com> writes:
> > The implementation is pretty ugly -- it clutters ExecuteStmt and Query
> > with fields that really do not belong there. Per previous discussion, I
> > think it would be better to refactor the CREATE TABLE AS implementation
> > to be essentially a CREATE TABLE followed by a INSERT ... SELECT.
>
> I kinda wonder why bother at all. I don't see any good reason why
> people shouldn't issue two statements.

The good reason is that CREATE plus INSERT SELECT is extremely bug prone
and very annoying to have to code SQL that way. CTAS is sent from on
high, IMHO, even without the performance optimisation.

Best Regards, Simon Riggs

On Tue, 2006-02-14 at 14:32 -0500, Kris Jurka wrote:
> This patch adds most of the options available for regular CREATE TABLE
> syntax to the CREATE TABLE x AS SELECT ... and AS EXECUTE ...

Barring any objections, I'll apply this later today or tomorrow.

-Neil

On Tue, 2006-02-14 at 14:32 -0500, Kris Jurka wrote:
> This patch adds most of the options available for regular CREATE TABLE
> syntax to the CREATE TABLE x AS SELECT ... and AS EXECUTE ...

Applied to HEAD, including the documentation updates posted separately.
Thanks for the patch.

-Neil