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.)

6 files changed, 142 insertions, 40 deletions

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 2a943f9c6a5..e5faf46a7a6 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;
+	Query	   *query;
 
-	/*
-	 * 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)
+	/* transform contained query */
+	query = transformStmt(pstate, stmt->query);
+	stmt->query = (Node *) query;
+
+	/* additional work needed for CREATE MATERIALIZED VIEW */
+	if (stmt->relkind == OBJECT_MATVIEW)
 	{
-		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);
-	}
+		/*
+		 * 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")));
 
-	/* transform contained query */
-	stmt->query = (Node *) transformStmt(pstate, stmt->query);
+		/*
+		 * 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 ed8502c6e4e..ec693734f51 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 78a4f13c711..1915210bab5 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 c98dee18b83..4f9168b074a 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 93aeab8d16f..82e088a38ba 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 0d2802a576a..46dc6724f47 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 */