add additional options to CREATE TABLE ... AS

Started by Kris Jurkaover 20 years ago8 messagespatches
Jump to latest
#1Kris Jurka
books@ejurka.com

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

Attachments:

create-table-options.patchtext/plain; charset=US-ASCII; name=create-table-options.patchDownload+234-56
#2Neil Conway
neilc@samurai.com
In reply to: Kris Jurka (#1)
Re: add additional options to CREATE TABLE ... AS

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

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Neil Conway (#2)
Re: add additional options to CREATE TABLE ... AS

Neil Conway <neilc@samurai.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

#4Kris Jurka
books@ejurka.com
In reply to: Kris Jurka (#1)
Re: add additional options to CREATE TABLE ... AS

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

Attachments:

create-as-doc.patchtext/plain; charset=US-ASCII; name=create-as-doc.patchDownload+85-12
#5Kris Jurka
books@ejurka.com
In reply to: Tom Lane (#3)
Re: add additional options to CREATE TABLE ... AS

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

#6Simon Riggs
simon@2ndQuadrant.com
In reply to: Tom Lane (#3)
Re: add additional options to CREATE TABLE ... AS

On Tue, 2006-02-14 at 15:38 -0500, Tom Lane wrote:

Neil Conway <neilc@samurai.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

#7Neil Conway
neilc@samurai.com
In reply to: Kris Jurka (#1)
Re: add additional options to CREATE TABLE ... AS

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

#8Neil Conway
neilc@samurai.com
In reply to: Kris Jurka (#1)
Re: add additional options to CREATE TABLE ... AS

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