Thread: 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
Attachment
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@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
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
Attachment
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@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
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