On 1/21/19 3:31 AM, Andreas Karlsson wrote:
Here is a a stab at refactoring this so the object creation does not
happen in a callback.
Rebased my patch on top of Andres's pluggable storage patches. Plus some
minor style changes.
Andreas
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 2bc8f928ea..7ef3794e08 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -55,16 +55,15 @@ typedef struct
{
DestReceiver pub; /* publicly-known function pointers */
IntoClause *into; /* target relation specification */
+ ObjectAddress reladdr; /* address of rel, for intorel_startup */
/* These fields are filled by intorel_startup: */
Relation rel; /* relation to write to */
- ObjectAddress reladdr; /* address of rel, for ExecCreateTableAs */
CommandId output_cid; /* cmin to insert in output tuples */
int hi_options; /* heap_insert performance options */
BulkInsertState bistate; /* bulk insert state */
} DR_intorel;
-/* utility functions for CTAS definition creation */
-static ObjectAddress create_ctas_internal(List *attrList, IntoClause *into);
+/* utility function for CTAS definition creation */
static ObjectAddress create_ctas_nodata(List *tlist, IntoClause *into);
/* DestReceiver routines for collecting data */
@@ -75,16 +74,18 @@ static void intorel_destroy(DestReceiver *self);
/*
- * create_ctas_internal
+ * create_ctas_nodata
*
- * Internal utility used for the creation of the definition of a relation
- * created via CREATE TABLE AS or a materialized view. Caller needs to
- * provide a list of attributes (ColumnDef nodes).
+ * Create CTAS or materialized view without the data, starting from the
+ * targetlist of the SELECT or view definition.
*/
static ObjectAddress
-create_ctas_internal(List *attrList, IntoClause *into)
+create_ctas_nodata(List *tlist, IntoClause *into)
{
- CreateStmt *create = makeNode(CreateStmt);
+ List *attrList;
+ ListCell *t,
+ *lc;
+ CreateStmt *create;
bool is_matview;
char relkind;
Datum toast_options;
@@ -96,71 +97,6 @@ create_ctas_internal(List *attrList, IntoClause *into)
relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;
/*
- * Create the target relation by faking up a CREATE TABLE parsetree and
- * passing it to DefineRelation.
- */
- create->relation = into->rel;
- create->tableElts = attrList;
- create->inhRelations = NIL;
- create->ofTypename = NULL;
- create->constraints = NIL;
- create->options = into->options;
- create->oncommit = into->onCommit;
- create->tablespacename = into->tableSpaceName;
- create->if_not_exists = false;
-
- /*
- * Create the relation. (This will error out if there's an existing view,
- * so we don't need more code to complain if "replace" is false.)
- */
- intoRelationAddr = DefineRelation(create, relkind, InvalidOid, NULL, NULL);
-
- /*
- * If necessary, create a TOAST table for the target table. Note that
- * NewRelationCreateToastTable ends with CommandCounterIncrement(), so
- * that the TOAST table will be visible for insertion.
- */
- CommandCounterIncrement();
-
- /* parse and validate reloptions for the toast table */
- toast_options = transformRelOptions((Datum) 0,
- create->options,
- "toast",
- validnsps,
- true, false);
-
- (void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
-
- NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
-
- /* Create the "view" part of a materialized view. */
- if (is_matview)
- {
- /* StoreViewQuery scribbles on tree, so make a copy */
- Query *query = (Query *) copyObject(into->viewQuery);
-
- StoreViewQuery(intoRelationAddr.objectId, query, false);
- CommandCounterIncrement();
- }
-
- return intoRelationAddr;
-}
-
-
-/*
- * create_ctas_nodata
- *
- * Create CTAS or materialized view when WITH NO DATA is used, starting from
- * the targetlist of the SELECT or view definition.
- */
-static ObjectAddress
-create_ctas_nodata(List *tlist, IntoClause *into)
-{
- List *attrList;
- ListCell *t,
- *lc;
-
- /*
* Build list of ColumnDefs from non-junk elements of the tlist. If a
* column name list was specified in CREATE TABLE AS, override the column
* names in the query. (Too few column names are OK, too many are not.)
@@ -213,8 +149,56 @@ create_ctas_nodata(List *tlist, IntoClause *into)
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("too many column names were specified")));
- /* Create the relation definition using the ColumnDef list */
- return create_ctas_internal(attrList, into);
+ /*
+ * Create the target relation by faking up a CREATE TABLE parsetree and
+ * passing it to DefineRelation.
+ */
+ create = makeNode(CreateStmt);
+ create->relation = into->rel;
+ create->tableElts = attrList;
+ create->inhRelations = NIL;
+ create->ofTypename = NULL;
+ create->constraints = NIL;
+ create->options = into->options;
+ create->oncommit = into->onCommit;
+ create->tablespacename = into->tableSpaceName;
+ create->if_not_exists = false;
+
+ /*
+ * Create the relation. (This will error out if there's an existing view,
+ * so we don't need more code to complain if "replace" is false.)
+ */
+ intoRelationAddr = DefineRelation(create, relkind, InvalidOid, NULL, NULL);
+
+ /*
+ * If necessary, create a TOAST table for the target table. Note that
+ * NewRelationCreateToastTable ends with CommandCounterIncrement(), so
+ * that the TOAST table will be visible for insertion.
+ */
+ CommandCounterIncrement();
+
+ /* parse and validate reloptions for the toast table */
+ toast_options = transformRelOptions((Datum) 0,
+ create->options,
+ "toast",
+ validnsps,
+ true, false);
+
+ (void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+
+ NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
+
+ /* Create the "view" part of a materialized view. */
+ if (is_matview)
+ {
+ /* StoreViewQuery scribbles on tree, so make a copy */
+ Query *query = (Query *) copyObject(into->viewQuery);
+
+ StoreViewQuery(intoRelationAddr.objectId, query, false);
+ CommandCounterIncrement();
+ }
+
+ return intoRelationAddr;
}
@@ -257,7 +241,7 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
/*
* Create the tuple receiver object and insert info it will need
*/
- dest = CreateIntoRelDestReceiver(into);
+ dest = CreateIntoRelDestReceiver();
/*
* The contained Query could be a SELECT, or an EXECUTE utility command.
@@ -278,33 +262,25 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
}
Assert(query->commandType == CMD_SELECT);
- /*
- * For materialized views, lock down security-restricted operations and
- * arrange to make GUC variable changes local to this command. This is
- * not necessary for security, but this keeps the behavior similar to
- * REFRESH MATERIALIZED VIEW. Otherwise, one could create a materialized
- * view not possible to refresh.
- */
- if (is_matview)
- {
- GetUserIdAndSecContext(&save_userid, &save_sec_context);
- SetUserIdAndSecContext(save_userid,
- save_sec_context | SECURITY_RESTRICTED_OPERATION);
- save_nestlevel = NewGUCNestLevel();
- }
+ DefineIntoRelForDestReceiver(dest, query->targetList, into);
- if (into->skipData)
+ if (!into->skipData)
{
/*
- * If WITH NO DATA was specified, do not go through the rewriter,
- * planner and executor. Just define the relation using a code path
- * similar to CREATE VIEW. This avoids dump/restore problems stemming
- * from running the planner before all dependencies are set up.
+ * For materialized views, lock down security-restricted operations and
+ * arrange to make GUC variable changes local to this command. This is
+ * not necessary for security, but this keeps the behavior similar to
+ * REFRESH MATERIALIZED VIEW. Otherwise, one could create a materialized
+ * view not possible to refresh.
*/
- address = create_ctas_nodata(query->targetList, into);
- }
- else
- {
+ if (is_matview)
+ {
+ GetUserIdAndSecContext(&save_userid, &save_sec_context);
+ SetUserIdAndSecContext(save_userid,
+ save_sec_context | SECURITY_RESTRICTED_OPERATION);
+ save_nestlevel = NewGUCNestLevel();
+ }
+
/*
* Parse analysis was done already, but we still have to run the rule
* rewriter. We do not do AcquireRewriteLocks: we assume the query
@@ -367,18 +343,18 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString,
FreeQueryDesc(queryDesc);
PopActiveSnapshot();
- }
- if (is_matview)
- {
- /* Roll back any GUC changes */
- AtEOXact_GUC(false, save_nestlevel);
+ if (is_matview)
+ {
+ /* Roll back any GUC changes */
+ AtEOXact_GUC(false, save_nestlevel);
- /* Restore userid and security context */
- SetUserIdAndSecContext(save_userid, save_sec_context);
+ /* Restore userid and security context */
+ SetUserIdAndSecContext(save_userid, save_sec_context);
+ }
}
- return address;
+ return ((DR_intorel *) dest)->reladdr;
}
/*
@@ -403,12 +379,11 @@ GetIntoRelEFlags(IntoClause *intoClause)
/*
* CreateIntoRelDestReceiver -- create a suitable DestReceiver object
*
- * intoClause will be NULL if called from CreateDestReceiver(), in which
- * case it has to be provided later. However, it is convenient to allow
- * self->into to be filled in immediately for other callers.
+ * The private fields are initialized later when CreateIntoRelDestReceiver is
+ * called to create the receiving relation.
*/
DestReceiver *
-CreateIntoRelDestReceiver(IntoClause *intoClause)
+CreateIntoRelDestReceiver(void)
{
DR_intorel *self = (DR_intorel *) palloc0(sizeof(DR_intorel));
@@ -417,13 +392,25 @@ CreateIntoRelDestReceiver(IntoClause *intoClause)
self->pub.rShutdown = intorel_shutdown;
self->pub.rDestroy = intorel_destroy;
self->pub.mydest = DestIntoRel;
- self->into = intoClause;
- /* other private fields will be set during intorel_startup */
+ /* private fields will be set by DefineIntoRelForDestReceiver */
return (DestReceiver *) self;
}
/*
+ * DefineIntoRelForDestReceiver -- create the receveiving relation
+ */
+void
+DefineIntoRelForDestReceiver(DestReceiver *dest, List *targetList, IntoClause *intoClause)
+{
+ DR_intorel *self = (DR_intorel *) dest;
+
+ self->into = intoClause;
+ self->reladdr = create_ctas_nodata(targetList, intoClause);
+ /* other private fields will be set during intorel_startup */
+}
+
+/*
* intorel_startup --- executor startup
*/
static void
@@ -431,77 +418,21 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
{
DR_intorel *myState = (DR_intorel *) self;
IntoClause *into = myState->into;
+ ObjectAddress intoRelationAddr = myState->reladdr;
bool is_matview;
char relkind;
- List *attrList;
- ObjectAddress intoRelationAddr;
Relation intoRelationDesc;
RangeTblEntry *rte;
- ListCell *lc;
int attnum;
- Assert(into != NULL); /* else somebody forgot to set it */
+ Assert(into != NULL); /* else somebody forgot to set it */
+ Assert(intoRelationAddr.classId != InvalidOid); /* else somebody forgot to set it */
/* This code supports both CREATE TABLE AS and CREATE MATERIALIZED VIEW */
is_matview = (into->viewQuery != NULL);
relkind = is_matview ? RELKIND_MATVIEW : RELKIND_RELATION;
/*
- * Build column definitions using "pre-cooked" type and collation info. If
- * a column name list was specified in CREATE TABLE AS, override the
- * column names derived from the query. (Too few column names are OK, too
- * many are not.)
- */
- attrList = NIL;
- lc = list_head(into->colNames);
- for (attnum = 0; attnum < typeinfo->natts; attnum++)
- {
- Form_pg_attribute attribute = TupleDescAttr(typeinfo, attnum);
- ColumnDef *col;
- char *colname;
-
- if (lc)
- {
- colname = strVal(lfirst(lc));
- lc = lnext(lc);
- }
- else
- colname = NameStr(attribute->attname);
-
- col = makeColumnDef(colname,
- attribute->atttypid,
- attribute->atttypmod,
- attribute->attcollation);
-
- /*
- * It's possible that the column is of a collatable type but the
- * collation could not be resolved, so double-check. (We must check
- * this here because DefineRelation would adopt the type's default
- * collation rather than complaining.)
- */
- if (!OidIsValid(col->collOid) &&
- type_is_collatable(col->typeName->typeOid))
- ereport(ERROR,
- (errcode(ERRCODE_INDETERMINATE_COLLATION),
- errmsg("no collation was derived for column \"%s\" with collatable type %s",
- col->colname,
- format_type_be(col->typeName->typeOid)),
- errhint("Use the COLLATE clause to set the collation explicitly.")));
-
- attrList = lappend(attrList, col);
- }
-
- if (lc != NULL)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("too many column names were specified")));
-
- /*
- * Actually create the target table
- */
- intoRelationAddr = create_ctas_internal(attrList, into);
-
- /*
* Finally we can open the target table
*/
intoRelationDesc = table_open(intoRelationAddr.objectId, AccessExclusiveLock);
@@ -549,7 +480,6 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
* Fill private fields of myState for use by later routines
*/
myState->rel = intoRelationDesc;
- myState->reladdr = intoRelationAddr;
myState->output_cid = GetCurrentCommandId(true);
/*
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index ae7f038203..7f443b9423 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -496,11 +496,14 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es,
UpdateActiveSnapshotCommandId();
/*
- * Normally we discard the query's output, but if explaining CREATE TABLE
+ * Normally we discard the query's output, but if executing CREATE TABLE
* AS, we'd better use the appropriate tuple receiver.
*/
- if (into)
- dest = CreateIntoRelDestReceiver(into);
+ if (into && es->analyze)
+ {
+ dest = CreateIntoRelDestReceiver();
+ DefineIntoRelForDestReceiver(dest, plannedstmt->planTree->targetlist, into);
+ }
else
dest = None_Receiver;
diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c
index a98c8362d7..2fd3f6bf60 100644
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -273,6 +273,8 @@ ExecuteQuery(ExecuteStmt *stmt, IntoClause *intoClause,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("prepared statement is not a SELECT")));
+ DefineIntoRelForDestReceiver(dest, pstmt->planTree->targetlist, intoClause);
+
/* Set appropriate eflags */
eflags = GetIntoRelEFlags(intoClause);
diff --git a/src/backend/tcop/dest.c b/src/backend/tcop/dest.c
index ee9e349a5b..ff278c2b81 100644
--- a/src/backend/tcop/dest.c
+++ b/src/backend/tcop/dest.c
@@ -139,7 +139,7 @@ CreateDestReceiver(CommandDest dest)
return CreateTuplestoreDestReceiver();
case DestIntoRel:
- return CreateIntoRelDestReceiver(NULL);
+ return CreateIntoRelDestReceiver();
case DestCopyOut:
return CreateCopyDestReceiver();
diff --git a/src/include/commands/createas.h b/src/include/commands/createas.h
index 1f02b149fb..09cbd00255 100644
--- a/src/include/commands/createas.h
+++ b/src/include/commands/createas.h
@@ -26,6 +26,8 @@ extern ObjectAddress ExecCreateTableAs(CreateTableAsStmt *stmt, const char *quer
extern int GetIntoRelEFlags(IntoClause *intoClause);
-extern DestReceiver *CreateIntoRelDestReceiver(IntoClause *intoClause);
+extern DestReceiver *CreateIntoRelDestReceiver(void);
+
+extern void DefineIntoRelForDestReceiver(DestReceiver *dest, List *targetList, IntoClause *intoClause);
#endif /* CREATEAS_H */
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 46deb55c67..6e12ae641b 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -3266,6 +3266,7 @@ SELECT b.relname,
matview_col1_idx | i | relfilenode has changed
pg_toast_TABLE | t | relfilenode is unchanged
pg_toast_TABLE_index | i | relfilenode has changed
+ reindex_before | r | relfilenode is unchanged
table1 | r | relfilenode is unchanged
table1_col1_seq | S | relfilenode is unchanged
table1_pkey | i | relfilenode has changed
@@ -3274,7 +3275,7 @@ SELECT b.relname,
table2_col2_idx | i | relfilenode has changed
table2_pkey | i | relfilenode has changed
view | v | relfilenode is unchanged
-(12 rows)
+(13 rows)
REINDEX SCHEMA schema_to_reindex;
BEGIN;
