From 0b337904213337db5026ef0a756a447588023935 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 12 Apr 2013 19:25:20 -0400 Subject: [PATCH] Clean up the mess around EXPLAIN and materialized views. Revert the matview-related changes in explain.c's API, as per recent complaint from Robert Haas. The reason for these appears to have been principally some ill-considered choices around having intorel_startup do what ought to be parse-time checking, plus a poor arrangement for passing it the view parsetree it needs to store into pg_rewrite when creating a materialized view. Do the latter by having parse analysis stick a copy into the IntoClause, instead of doing it at runtime. (On the whole, I seriously question the choice to represent CREATE MATERIALIZED VIEW as a variant of SELECT INTO/CREATE TABLE AS, because that means injecting even more complexity into what was already a horrid legacy kluge. However, I didn't go so far as to rethink that choice ... yet.) I also moved several error checks into matview parse analysis, and made the check for external Params in a matview more accurate. In passing, clean things up a bit more around interpretOidsOption(), and fix things so that we can use that to force no-oids for views, sequences, etc, thereby eliminating the need to cons up "oids = false" options when creating them. catversion bump due to change in IntoClause. (I wonder though if we really need readfuncs/outfuncs support for IntoClause anymore.) --- src/backend/commands/createas.c | 142 +++++++++------------------- src/backend/commands/explain.c | 37 ++++---- src/backend/commands/prepare.c | 2 +- src/backend/commands/sequence.c | 2 +- src/backend/commands/tablecmds.c | 54 +---------- src/backend/commands/typecmds.c | 2 +- src/backend/commands/view.c | 1 - src/backend/nodes/copyfuncs.c | 2 +- src/backend/nodes/equalfuncs.c | 2 +- src/backend/nodes/nodeFuncs.c | 3 + src/backend/nodes/outfuncs.c | 2 +- src/backend/nodes/readfuncs.c | 2 +- src/backend/parser/analyze.c | 64 +++++++++---- src/backend/parser/gram.y | 13 +-- src/backend/parser/parse_clause.c | 18 ++-- src/backend/parser/parse_param.c | 36 +++++++ src/backend/parser/parse_relation.c | 49 ++++++++++ src/backend/parser/parse_utilcmd.c | 4 +- src/include/catalog/catversion.h | 2 +- src/include/commands/createas.h | 4 - src/include/commands/explain.h | 5 +- src/include/commands/tablecmds.h | 2 - src/include/nodes/primnodes.h | 6 +- src/include/parser/parse_clause.h | 2 +- src/include/parser/parse_param.h | 1 + src/include/parser/parse_relation.h | 1 + 26 files changed, 231 insertions(+), 227 deletions(-) diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c index 079fafa06f..94a5fa755e 100644 --- a/src/backend/commands/createas.c +++ b/src/backend/commands/createas.c @@ -1,14 +1,14 @@ /*------------------------------------------------------------------------- * * createas.c - * Execution of CREATE TABLE ... AS, a/k/a SELECT INTO + * Execution of CREATE TABLE ... AS, a/k/a SELECT INTO. * Since CREATE MATERIALIZED VIEW shares syntax and most behaviors, - * implement that here, too. + * we implement that here, too. * * We implement this by diverting the query's normal output to a * specialized DestReceiver type. * - * Formerly, this command was implemented as a variant of SELECT, which led + * Formerly, CTAS was implemented as a variant of SELECT, which led * to assorted legacy behaviors that we still try to preserve, notably that * we must return a tuples-processed count in the completionTag. * @@ -33,7 +33,6 @@ #include "commands/prepare.h" #include "commands/tablecmds.h" #include "commands/view.h" -#include "parser/analyze.h" #include "parser/parse_clause.h" #include "rewrite/rewriteHandler.h" #include "storage/smgr.h" @@ -48,7 +47,6 @@ typedef struct { DestReceiver pub; /* publicly-known function pointers */ IntoClause *into; /* target relation specification */ - Query *viewParse; /* the query which defines/populates data */ /* These fields are filled by intorel_startup: */ Relation rel; /* relation to write to */ CommandId output_cid; /* cmin to insert in output tuples */ @@ -62,62 +60,6 @@ static void intorel_shutdown(DestReceiver *self); static void intorel_destroy(DestReceiver *self); -/* - * Common setup needed by both normal execution and EXPLAIN ANALYZE. - */ -Query * -SetupForCreateTableAs(Query *query, IntoClause *into, const char *queryString, - ParamListInfo params, DestReceiver *dest) -{ - List *rewritten; - Query *viewParse = NULL; - - Assert(query->commandType == CMD_SELECT); - - if (into->relkind == RELKIND_MATVIEW) - viewParse = (Query *) parse_analyze((Node *) copyObject(query), - queryString, NULL, 0)->utilityStmt; - - /* - * Parse analysis was done already, but we still have to run the rule - * rewriter. We do not do AcquireRewriteLocks: we assume the query either - * came straight from the parser, or suitable locks were acquired by - * plancache.c. - * - * Because the rewriter and planner tend to scribble on the input, we make - * a preliminary copy of the source querytree. This prevents problems in - * the case that CTAS is in a portal or plpgsql function and is executed - * repeatedly. (See also the same hack in EXPLAIN and PREPARE.) - */ - rewritten = QueryRewrite((Query *) copyObject(query)); - - /* SELECT should never rewrite to more or less than one SELECT query */ - if (list_length(rewritten) != 1) - elog(ERROR, "unexpected rewrite result for CREATE TABLE AS SELECT"); - query = (Query *) linitial(rewritten); - - Assert(query->commandType == CMD_SELECT); - - /* Save the query after rewrite but before planning. */ - ((DR_intorel *) dest)->viewParse = viewParse; - ((DR_intorel *) dest)->into = into; - - if (into->relkind == RELKIND_MATVIEW) - { - /* - * A materialized view would either need to save parameters for use in - * maintaining or loading the data or prohibit them entirely. The - * latter seems safer and more sane. - */ - if (params != NULL && params->numParams > 0) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("materialized views may not be defined using bound parameters"))); - } - - return query; -} - /* * ExecCreateTableAs -- execute a CREATE TABLE AS command */ @@ -128,6 +70,7 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString, Query *query = (Query *) stmt->query; IntoClause *into = stmt->into; DestReceiver *dest; + List *rewritten; PlannedStmt *plan; QueryDesc *queryDesc; ScanDirection dir; @@ -151,8 +94,26 @@ ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString, return; } + Assert(query->commandType == CMD_SELECT); - query = SetupForCreateTableAs(query, into, queryString, params, dest); + /* + * Parse analysis was done already, but we still have to run the rule + * rewriter. We do not do AcquireRewriteLocks: we assume the query either + * came straight from the parser, or suitable locks were acquired by + * plancache.c. + * + * Because the rewriter and planner tend to scribble on the input, we make + * a preliminary copy of the source querytree. This prevents problems in + * the case that CTAS is in a portal or plpgsql function and is executed + * repeatedly. (See also the same hack in EXPLAIN and PREPARE.) + */ + rewritten = QueryRewrite((Query *) copyObject(query)); + + /* SELECT should never rewrite to more or less than one SELECT query */ + if (list_length(rewritten) != 1) + elog(ERROR, "unexpected rewrite result for CREATE TABLE AS SELECT"); + query = (Query *) linitial(rewritten); + Assert(query->commandType == CMD_SELECT); /* plan the query */ plan = pg_plan_query(query, 0, params); @@ -213,20 +174,20 @@ int GetIntoRelEFlags(IntoClause *intoClause) { int flags; + /* * We need to tell the executor whether it has to produce OIDs or not, * because it doesn't have enough information to do so itself (since we * can't build the target relation until after ExecutorStart). + * + * Disallow the OIDS option for materialized views. */ - if (interpretOidsOption(intoClause->options, intoClause->relkind)) + if (interpretOidsOption(intoClause->options, + (intoClause->viewQuery == NULL))) flags = EXEC_FLAG_WITH_OIDS; else flags = EXEC_FLAG_WITHOUT_OIDS; - Assert(intoClause->relkind != RELKIND_MATVIEW || - (flags & (EXEC_FLAG_WITH_OIDS | EXEC_FLAG_WITHOUT_OIDS)) == - EXEC_FLAG_WITHOUT_OIDS); - if (intoClause->skipData) flags |= EXEC_FLAG_WITH_NO_DATA; @@ -264,6 +225,8 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) { DR_intorel *myState = (DR_intorel *) self; IntoClause *into = myState->into; + bool is_matview; + char relkind; CreateStmt *create; Oid intoRelationId; Relation intoRelationDesc; @@ -275,6 +238,10 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) Assert(into != NULL); /* 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; + /* * Create the target relation by faking up a CREATE TABLE parsetree and * passing it to DefineRelation. @@ -352,38 +319,12 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) if (lc != NULL) ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("too many column names are specified"))); - - /* - * Enforce validations needed for materialized views only. - */ - if (into->relkind == RELKIND_MATVIEW) - { - /* - * Prohibit a data-modifying CTE in the query used to create a - * materialized view. It's not sufficiently clear what the user would - * want to happen if the MV is refreshed or incrementally maintained. - */ - if (myState->viewParse->hasModifyingCTE) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("materialized views must not use data-modifying statements in WITH"))); - - /* - * Check whether any temporary database objects are used in the - * creation query. It would be hard to refresh data or incrementally - * maintain it if a source disappeared. - */ - if (isQueryUsingTempRelation(myState->viewParse)) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("materialized views must not use temporary tables or views"))); - } + errmsg("too many column names were specified"))); /* * Actually create the target table */ - intoRelationId = DefineRelation(create, into->relkind, InvalidOid); + intoRelationId = DefineRelation(create, relkind, InvalidOid); /* * If necessary, create a TOAST table for the target table. Note that @@ -404,9 +345,12 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) AlterTableCreateToastTable(intoRelationId, toast_options); /* Create the "view" part of a materialized view. */ - if (into->relkind == RELKIND_MATVIEW) + if (is_matview) { - StoreViewQuery(intoRelationId, myState->viewParse, false); + /* StoreViewQuery scribbles on tree, so make a copy */ + Query *query = (Query *) copyObject(into->viewQuery); + + StoreViewQuery(intoRelationId, query, false); CommandCounterIncrement(); } @@ -415,7 +359,7 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) */ intoRelationDesc = heap_open(intoRelationId, AccessExclusiveLock); - if (into->relkind == RELKIND_MATVIEW && !into->skipData) + if (is_matview && !into->skipData) /* Make sure the heap looks good even if no rows are written. */ SetMatViewToPopulated(intoRelationDesc); @@ -428,7 +372,7 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo) rte = makeNode(RangeTblEntry); rte->rtekind = RTE_RELATION; rte->relid = intoRelationId; - rte->relkind = into->relkind; + rte->relkind = relkind; rte->isResultRel = true; rte->requiredPerms = ACL_INSERT; diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c index 67b97eef87..38ce0efe03 100644 --- a/src/backend/commands/explain.c +++ b/src/backend/commands/explain.c @@ -47,7 +47,7 @@ explain_get_index_name_hook_type explain_get_index_name_hook = NULL; #define X_NOWHITESPACE 4 static void ExplainOneQuery(Query *query, IntoClause *into, ExplainState *es, - const char *queryString, DestReceiver *dest, ParamListInfo params); + const char *queryString, ParamListInfo params); static void report_triggers(ResultRelInfo *rInfo, bool show_relname, ExplainState *es); static double elapsed_time(instr_time *starttime); @@ -219,7 +219,7 @@ ExplainQuery(ExplainStmt *stmt, const char *queryString, foreach(l, rewritten) { ExplainOneQuery((Query *) lfirst(l), NULL, &es, - queryString, None_Receiver, params); + queryString, params); /* Separate plans with an appropriate separator */ if (lnext(l) != NULL) @@ -300,8 +300,7 @@ ExplainResultDesc(ExplainStmt *stmt) */ static void ExplainOneQuery(Query *query, IntoClause *into, ExplainState *es, - const char *queryString, DestReceiver *dest, - ParamListInfo params) + const char *queryString, ParamListInfo params) { /* planner will not cope with utility statements */ if (query->commandType == CMD_UTILITY) @@ -312,7 +311,7 @@ ExplainOneQuery(Query *query, IntoClause *into, ExplainState *es, /* if an advisor plugin is present, let it manage things */ if (ExplainOneQuery_hook) - (*ExplainOneQuery_hook) (query, into, es, queryString, dest, params); + (*ExplainOneQuery_hook) (query, into, es, queryString, params); else { PlannedStmt *plan; @@ -321,7 +320,7 @@ ExplainOneQuery(Query *query, IntoClause *into, ExplainState *es, plan = pg_plan_query(query, 0, params); /* run it (if needed) and produce output */ - ExplainOnePlan(plan, into, es, queryString, dest, params); + ExplainOnePlan(plan, into, es, queryString, params); } } @@ -345,23 +344,19 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es, if (IsA(utilityStmt, CreateTableAsStmt)) { - DestReceiver *dest; - /* * We have to rewrite the contained SELECT and then pass it back to * ExplainOneQuery. It's probably not really necessary to copy the * contained parsetree another time, but let's be safe. */ CreateTableAsStmt *ctas = (CreateTableAsStmt *) utilityStmt; - Query *query = (Query *) ctas->query; - - dest = CreateIntoRelDestReceiver(into); + List *rewritten; Assert(IsA(ctas->query, Query)); - - query = SetupForCreateTableAs(query, ctas->into, queryString, params, dest); - - ExplainOneQuery(query, ctas->into, es, queryString, dest, params); + rewritten = QueryRewrite((Query *) copyObject(ctas->query)); + Assert(list_length(rewritten) == 1); + ExplainOneQuery((Query *) linitial(rewritten), ctas->into, es, + queryString, params); } else if (IsA(utilityStmt, ExecuteStmt)) ExplainExecuteQuery((ExecuteStmt *) utilityStmt, into, es, @@ -402,8 +397,9 @@ ExplainOneUtility(Node *utilityStmt, IntoClause *into, ExplainState *es, */ void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es, - const char *queryString, DestReceiver *dest, ParamListInfo params) + const char *queryString, ParamListInfo params) { + DestReceiver *dest; QueryDesc *queryDesc; instr_time starttime; double totaltime = 0; @@ -427,6 +423,15 @@ ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, ExplainState *es, PushCopiedSnapshot(GetActiveSnapshot()); UpdateActiveSnapshotCommandId(); + /* + * Normally we discard the query's output, but if explaining CREATE TABLE + * AS, we'd better use the appropriate tuple receiver. + */ + if (into) + dest = CreateIntoRelDestReceiver(into); + else + dest = None_Receiver; + /* Create a QueryDesc for the query */ queryDesc = CreateQueryDesc(plannedstmt, queryString, GetActiveSnapshot(), InvalidSnapshot, diff --git a/src/backend/commands/prepare.c b/src/backend/commands/prepare.c index c79bc020c2..62208eb995 100644 --- a/src/backend/commands/prepare.c +++ b/src/backend/commands/prepare.c @@ -665,7 +665,7 @@ ExplainExecuteQuery(ExecuteStmt *execstmt, IntoClause *into, ExplainState *es, PlannedStmt *pstmt = (PlannedStmt *) lfirst(p); if (IsA(pstmt, PlannedStmt)) - ExplainOnePlan(pstmt, into, es, query_string, None_Receiver, paramLI); + ExplainOnePlan(pstmt, into, es, query_string, paramLI); else ExplainOneUtility((Node *) pstmt, into, es, query_string, paramLI); diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index c6add68b9f..ff855d60e5 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -210,7 +210,7 @@ DefineSequence(CreateSeqStmt *seq) stmt->relation = seq->sequence; stmt->inhRelations = NIL; stmt->constraints = NIL; - stmt->options = list_make1(defWithOids(false)); + stmt->options = NIL; stmt->oncommit = ONCOMMIT_NOOP; stmt->tablespacename = NULL; stmt->if_not_exists = false; diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 81c1199742..cd2c961008 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -407,8 +407,6 @@ static void RangeVarCallbackForDropRelation(const RangeVar *rel, Oid relOid, static void RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid, void *arg); -static bool isQueryUsingTempRelation_walker(Node *node, void *context); - /* ---------------------------------------------------------------- * DefineRelation @@ -560,7 +558,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId) */ descriptor = BuildDescForRelation(schema); - localHasOids = interpretOidsOption(stmt->options, relkind); + localHasOids = interpretOidsOption(stmt->options, + (relkind == RELKIND_RELATION || + relkind == RELKIND_FOREIGN_TABLE)); descriptor->tdhasoid = (localHasOids || parentOidCount > 0); /* @@ -10538,51 +10538,3 @@ RangeVarCallbackForAlterRelation(const RangeVar *rv, Oid relid, Oid oldrelid, ReleaseSysCache(tuple); } - -/* - * Returns true iff any relation underlying this query is a temporary database - * object (table, view, or materialized view). - * - */ -bool -isQueryUsingTempRelation(Query *query) -{ - return isQueryUsingTempRelation_walker((Node *) query, NULL); -} - -static bool -isQueryUsingTempRelation_walker(Node *node, void *context) -{ - if (node == NULL) - return false; - - if (IsA(node, Query)) - { - Query *query = (Query *) node; - ListCell *rtable; - - foreach(rtable, query->rtable) - { - RangeTblEntry *rte = lfirst(rtable); - - if (rte->rtekind == RTE_RELATION) - { - Relation rel = heap_open(rte->relid, AccessShareLock); - char relpersistence = rel->rd_rel->relpersistence; - - heap_close(rel, AccessShareLock); - if (relpersistence == RELPERSISTENCE_TEMP) - return true; - } - } - - return query_tree_walker(query, - isQueryUsingTempRelation_walker, - context, - QTW_IGNORE_JOINALIASES); - } - - return expression_tree_walker(node, - isQueryUsingTempRelation_walker, - context); -} diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 2c4c6cbced..9efe24417e 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -2032,7 +2032,7 @@ DefineCompositeType(RangeVar *typevar, List *coldeflist) createStmt->tableElts = coldeflist; createStmt->inhRelations = NIL; createStmt->constraints = NIL; - createStmt->options = list_make1(defWithOids(false)); + createStmt->options = NIL; createStmt->oncommit = ONCOMMIT_NOOP; createStmt->tablespacename = NULL; createStmt->if_not_exists = false; diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c index aba6944bdf..6186a84155 100644 --- a/src/backend/commands/view.c +++ b/src/backend/commands/view.c @@ -208,7 +208,6 @@ DefineVirtualRelation(RangeVar *relation, List *tlist, bool replace, createStmt->inhRelations = NIL; createStmt->constraints = NIL; createStmt->options = options; - createStmt->options = lappend(options, defWithOids(false)); createStmt->oncommit = ONCOMMIT_NOOP; createStmt->tablespacename = NULL; createStmt->if_not_exists = false; diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index fd3823a36e..6bfbbf4213 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -1032,8 +1032,8 @@ _copyIntoClause(const IntoClause *from) COPY_NODE_FIELD(options); COPY_SCALAR_FIELD(onCommit); COPY_STRING_FIELD(tableSpaceName); + COPY_NODE_FIELD(viewQuery); COPY_SCALAR_FIELD(skipData); - COPY_SCALAR_FIELD(relkind); return newnode; } diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 085cd5bee1..7b49f0afb9 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -123,8 +123,8 @@ _equalIntoClause(const IntoClause *a, const IntoClause *b) COMPARE_NODE_FIELD(options); COMPARE_SCALAR_FIELD(onCommit); COMPARE_STRING_FIELD(tableSpaceName); + COMPARE_NODE_FIELD(viewQuery); COMPARE_SCALAR_FIELD(skipData); - COMPARE_SCALAR_FIELD(relkind); return true; } diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c index dcf12b42b7..42d6621a82 100644 --- a/src/backend/nodes/nodeFuncs.c +++ b/src/backend/nodes/nodeFuncs.c @@ -2829,6 +2829,9 @@ raw_expression_tree_walker(Node *node, if (walker(into->rel, context)) return true; /* colNames, options are deemed uninteresting */ + /* viewQuery should be null in raw parsetree, but check it */ + if (walker(into->viewQuery, context)) + return true; } break; case T_List: diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index d8ce5753a4..bd47ddd0a2 100644 --- a/src/backend/nodes/outfuncs.c +++ b/src/backend/nodes/outfuncs.c @@ -893,8 +893,8 @@ _outIntoClause(StringInfo str, const IntoClause *node) WRITE_NODE_FIELD(options); WRITE_ENUM_FIELD(onCommit, OnCommitAction); WRITE_STRING_FIELD(tableSpaceName); + WRITE_NODE_FIELD(viewQuery); WRITE_BOOL_FIELD(skipData); - WRITE_CHAR_FIELD(relkind); } static void diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c index cee67f2eb9..f275a31e3c 100644 --- a/src/backend/nodes/readfuncs.c +++ b/src/backend/nodes/readfuncs.c @@ -394,8 +394,8 @@ _readIntoClause(void) READ_NODE_FIELD(options); READ_ENUM_FIELD(onCommit, OnCommitAction); READ_STRING_FIELD(tableSpaceName); + READ_NODE_FIELD(viewQuery); READ_BOOL_FIELD(skipData); - READ_CHAR_FIELD(relkind); READ_DONE(); } diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 2a943f9c6a..e5faf46a7a 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -2132,27 +2132,53 @@ static Query * transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt) { Query *result; - - /* - * Set relkind in IntoClause based on statement relkind. These are - * different types, because the parser users the ObjectType enumeration - * and the executor uses RELKIND_* defines. - */ - switch (stmt->relkind) - { - case (OBJECT_TABLE): - stmt->into->relkind = RELKIND_RELATION; - break; - case (OBJECT_MATVIEW): - stmt->into->relkind = RELKIND_MATVIEW; - break; - default: - elog(ERROR, "unrecognized object relkind: %d", - (int) stmt->relkind); - } + Query *query; /* transform contained query */ - stmt->query = (Node *) transformStmt(pstate, stmt->query); + query = transformStmt(pstate, stmt->query); + stmt->query = (Node *) query; + + /* additional work needed for CREATE MATERIALIZED VIEW */ + if (stmt->relkind == OBJECT_MATVIEW) + { + /* + * Prohibit a data-modifying CTE in the query used to create a + * materialized view. It's not sufficiently clear what the user would + * want to happen if the MV is refreshed or incrementally maintained. + */ + if (query->hasModifyingCTE) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialized views must not use data-modifying statements in WITH"))); + + /* + * Check whether any temporary database objects are used in the + * creation query. It would be hard to refresh data or incrementally + * maintain it if a source disappeared. + */ + if (isQueryUsingTempRelation(query)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialized views must not use temporary tables or views"))); + + /* + * A materialized view would either need to save parameters for use in + * maintaining/loading the data or prohibit them entirely. The latter + * seems safer and more sane. + */ + if (query_contains_extern_params(query)) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("materialized views may not be defined using bound parameters"))); + + /* + * At runtime, we'll need a copy of the parsed-but-not-rewritten Query + * for purposes of creating the view's ON SELECT rule. We stash that + * in the IntoClause because that's where intorel_startup() can + * conveniently get it from. + */ + stmt->into->viewQuery = copyObject(query); + } /* represent the command as a utility Query */ result = makeNode(Query); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index ed8502c6e4..ec693734f5 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -121,13 +121,6 @@ typedef struct PrivTarget #define CAS_NOT_VALID 0x10 #define CAS_NO_INHERIT 0x20 -/* - * In the IntoClause structure there is a char value which will eventually be - * set to RELKIND_RELATION or RELKIND_MATVIEW based on the relkind field in - * the statement-level structure, which is an ObjectType. Define the default - * here, which should always be overridden later. - */ -#define INTO_CLAUSE_RELKIND_DEFAULT '\0' #define parser_yyerror(msg) scanner_yyerror(msg, yyscanner) #define parser_errposition(pos) scanner_errposition(pos, yyscanner) @@ -3231,8 +3224,8 @@ create_as_target: $$->options = $3; $$->onCommit = $4; $$->tableSpaceName = $5; + $$->viewQuery = NULL; $$->skipData = false; /* might get changed later */ - $$->relkind = INTO_CLAUSE_RELKIND_DEFAULT; } ; @@ -3274,8 +3267,8 @@ create_mv_target: $$->options = $3; $$->onCommit = ONCOMMIT_NOOP; $$->tableSpaceName = $4; + $$->viewQuery = NULL; /* filled at analysis time */ $$->skipData = false; /* might get changed later */ - $$->relkind = INTO_CLAUSE_RELKIND_DEFAULT; } ; @@ -9285,8 +9278,8 @@ into_clause: $$->options = NIL; $$->onCommit = ONCOMMIT_NOOP; $$->tableSpaceName = NULL; + $$->viewQuery = NULL; $$->skipData = false; - $$->relkind = INTO_CLAUSE_RELKIND_DEFAULT; } | /*EMPTY*/ { $$ = NULL; } diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c index 78a4f13c71..1915210bab 100644 --- a/src/backend/parser/parse_clause.c +++ b/src/backend/parser/parse_clause.c @@ -244,13 +244,12 @@ interpretInhOption(InhOption inhOpt) * parsing the query string because the return value can depend upon the * default_with_oids GUC var. * - * Materialized views are handled here rather than reloptions.c because that - * code explicitly punts checking for oids to here. We prohibit any explicit - * specification of the oids option for a materialized view, and indicate that - * oids are not needed if we don't get an error. + * In some situations, we want to reject an OIDS option even if it's present. + * That's (rather messily) handled here rather than reloptions.c, because that + * code explicitly punts checking for oids to here. */ bool -interpretOidsOption(List *defList, char relkind) +interpretOidsOption(List *defList, bool allowOids) { ListCell *cell; @@ -262,16 +261,17 @@ interpretOidsOption(List *defList, char relkind) if (def->defnamespace == NULL && pg_strcasecmp(def->defname, "oids") == 0) { - if (relkind == RELKIND_MATVIEW) + if (!allowOids) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("unrecognized parameter \"%s\"", "oids"))); - + errmsg("unrecognized parameter \"%s\"", + def->defname))); return defGetBoolean(def); } } - if (relkind == RELKIND_MATVIEW) + /* Force no-OIDS result if caller disallows OIDS. */ + if (!allowOids) return false; /* OIDS option was not specified, so use default. */ diff --git a/src/backend/parser/parse_param.c b/src/backend/parser/parse_param.c index c98dee18b8..4f9168b074 100644 --- a/src/backend/parser/parse_param.c +++ b/src/backend/parser/parse_param.c @@ -57,6 +57,7 @@ static Node *variable_coerce_param_hook(ParseState *pstate, Param *param, Oid targetTypeId, int32 targetTypeMod, int location); static bool check_parameter_resolution_walker(Node *node, ParseState *pstate); +static bool query_contains_extern_params_walker(Node *node, void *context); /* @@ -316,3 +317,38 @@ check_parameter_resolution_walker(Node *node, ParseState *pstate) return expression_tree_walker(node, check_parameter_resolution_walker, (void *) pstate); } + +/* + * Check to see if a fully-parsed query tree contains any PARAM_EXTERN Params. + */ +bool +query_contains_extern_params(Query *query) +{ + return query_tree_walker(query, + query_contains_extern_params_walker, + NULL, 0); +} + +static bool +query_contains_extern_params_walker(Node *node, void *context) +{ + if (node == NULL) + return false; + if (IsA(node, Param)) + { + Param *param = (Param *) node; + + if (param->paramkind == PARAM_EXTERN) + return true; + return false; + } + if (IsA(node, Query)) + { + /* Recurse into RTE subquery or not-yet-planned sublink subquery */ + return query_tree_walker((Query *) node, + query_contains_extern_params_walker, + context, 0); + } + return expression_tree_walker(node, query_contains_extern_params_walker, + context); +} diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c index 93aeab8d16..82e088a38b 100644 --- a/src/backend/parser/parse_relation.c +++ b/src/backend/parser/parse_relation.c @@ -48,6 +48,7 @@ static void expandTupleDesc(TupleDesc tupdesc, Alias *eref, int location, bool include_dropped, List **colnames, List **colvars); static int specialAttNum(const char *attname); +static bool isQueryUsingTempRelation_walker(Node *node, void *context); /* @@ -2615,3 +2616,51 @@ errorMissingColumn(ParseState *pstate, colname, rte->eref->aliasname) : 0, parser_errposition(pstate, location))); } + + +/* + * Examine a fully-parsed query, and return TRUE iff any relation underlying + * the query is a temporary relation (table, view, or materialized view). + */ +bool +isQueryUsingTempRelation(Query *query) +{ + return isQueryUsingTempRelation_walker((Node *) query, NULL); +} + +static bool +isQueryUsingTempRelation_walker(Node *node, void *context) +{ + if (node == NULL) + return false; + + if (IsA(node, Query)) + { + Query *query = (Query *) node; + ListCell *rtable; + + foreach(rtable, query->rtable) + { + RangeTblEntry *rte = lfirst(rtable); + + if (rte->rtekind == RTE_RELATION) + { + Relation rel = heap_open(rte->relid, AccessShareLock); + char relpersistence = rel->rd_rel->relpersistence; + + heap_close(rel, AccessShareLock); + if (relpersistence == RELPERSISTENCE_TEMP) + return true; + } + } + + return query_tree_walker(query, + isQueryUsingTempRelation_walker, + context, + QTW_IGNORE_JOINALIASES); + } + + return expression_tree_walker(node, + isQueryUsingTempRelation_walker, + context); +} diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 0d2802a576..46dc6724f4 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -199,14 +199,11 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) { cxt.stmtType = "CREATE FOREIGN TABLE"; cxt.isforeign = true; - cxt.hasoids = interpretOidsOption(stmt->options, - RELKIND_FOREIGN_TABLE); } else { cxt.stmtType = "CREATE TABLE"; cxt.isforeign = false; - cxt.hasoids = interpretOidsOption(stmt->options, RELKIND_RELATION); } cxt.relation = stmt->relation; cxt.rel = NULL; @@ -220,6 +217,7 @@ transformCreateStmt(CreateStmt *stmt, const char *queryString) cxt.blist = NIL; cxt.alist = NIL; cxt.pkey = NULL; + cxt.hasoids = interpretOidsOption(stmt->options, true); Assert(!stmt->ofTypename || !stmt->inhRelations); /* grammar enforces */ diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 39216c029f..a77f67ca05 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 201304071 +#define CATALOG_VERSION_NO 201304121 #endif diff --git a/src/include/commands/createas.h b/src/include/commands/createas.h index 012334b42e..2ac718762b 100644 --- a/src/include/commands/createas.h +++ b/src/include/commands/createas.h @@ -19,10 +19,6 @@ #include "tcop/dest.h" -extern Query *SetupForCreateTableAs(Query *query, IntoClause *into, - const char *queryString, - ParamListInfo params, DestReceiver *dest); - extern void ExecCreateTableAs(CreateTableAsStmt *stmt, const char *queryString, ParamListInfo params, char *completionTag); diff --git a/src/include/commands/explain.h b/src/include/commands/explain.h index 4b740d5bfd..ca213d7f70 100644 --- a/src/include/commands/explain.h +++ b/src/include/commands/explain.h @@ -47,7 +47,6 @@ typedef void (*ExplainOneQuery_hook_type) (Query *query, IntoClause *into, ExplainState *es, const char *queryString, - DestReceiver *dest, ParamListInfo params); extern PGDLLIMPORT ExplainOneQuery_hook_type ExplainOneQuery_hook; @@ -68,8 +67,8 @@ extern void ExplainOneUtility(Node *utilityStmt, IntoClause *into, const char *queryString, ParamListInfo params); extern void ExplainOnePlan(PlannedStmt *plannedstmt, IntoClause *into, - ExplainState *es, const char *queryString, - DestReceiver *dest, ParamListInfo params); + ExplainState *es, + const char *queryString, ParamListInfo params); extern void ExplainPrintPlan(ExplainState *es, QueryDesc *queryDesc); diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h index 7b86c444f6..c07603b43d 100644 --- a/src/include/commands/tablecmds.h +++ b/src/include/commands/tablecmds.h @@ -78,6 +78,4 @@ extern void AtEOSubXact_on_commit_actions(bool isCommit, extern void RangeVarCallbackOwnsTable(const RangeVar *relation, Oid relId, Oid oldRelId, void *arg); -extern bool isQueryUsingTempRelation(Query *query); - #endif /* TABLECMDS_H */ diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h index 27d4e4cd67..153957fbfc 100644 --- a/src/include/nodes/primnodes.h +++ b/src/include/nodes/primnodes.h @@ -82,6 +82,10 @@ typedef struct RangeVar /* * IntoClause - target information for SELECT INTO, CREATE TABLE AS, and * CREATE MATERIALIZED VIEW + * + * For CREATE MATERIALIZED VIEW, viewQuery is the parsed-but-not-rewritten + * SELECT Query for the view; otherwise it's NULL. (Although it's actually + * Query*, we declare it as Node* to avoid a forward reference.) */ typedef struct IntoClause { @@ -92,8 +96,8 @@ typedef struct IntoClause List *options; /* options from WITH clause */ OnCommitAction onCommit; /* what do we do at COMMIT? */ char *tableSpaceName; /* table space to use, or NULL */ + Node *viewQuery; /* materialized view's SELECT query */ bool skipData; /* true for WITH NO DATA */ - char relkind; /* RELKIND_RELATION or RELKIND_MATVIEW */ } IntoClause; diff --git a/src/include/parser/parse_clause.h b/src/include/parser/parse_clause.h index 0bccb1cd64..9bdb03347a 100644 --- a/src/include/parser/parse_clause.h +++ b/src/include/parser/parse_clause.h @@ -20,7 +20,7 @@ extern void transformFromClause(ParseState *pstate, List *frmList); extern int setTargetTable(ParseState *pstate, RangeVar *relation, bool inh, bool alsoSource, AclMode requiredPerms); extern bool interpretInhOption(InhOption inhOpt); -extern bool interpretOidsOption(List *defList, char relkind); +extern bool interpretOidsOption(List *defList, bool allowOids); extern Node *transformWhereClause(ParseState *pstate, Node *clause, ParseExprKind exprKind, const char *constructName); diff --git a/src/include/parser/parse_param.h b/src/include/parser/parse_param.h index 6df2977a29..a6a9a08eb6 100644 --- a/src/include/parser/parse_param.h +++ b/src/include/parser/parse_param.h @@ -20,5 +20,6 @@ extern void parse_fixed_parameters(ParseState *pstate, extern void parse_variable_parameters(ParseState *pstate, Oid **paramTypes, int *numParams); extern void check_variable_parameters(ParseState *pstate, Query *query); +extern bool query_contains_extern_params(Query *query); #endif /* PARSE_PARAM_H */ diff --git a/src/include/parser/parse_relation.h b/src/include/parser/parse_relation.h index c9e4eafda9..d513b22e18 100644 --- a/src/include/parser/parse_relation.h +++ b/src/include/parser/parse_relation.h @@ -96,5 +96,6 @@ extern int attnameAttNum(Relation rd, const char *attname, bool sysColOK); extern Name attnumAttName(Relation rd, int attid); extern Oid attnumTypeId(Relation rd, int attid); extern Oid attnumCollationId(Relation rd, int attid); +extern bool isQueryUsingTempRelation(Query *query); #endif /* PARSE_RELATION_H */