Incidental cleanup of matviews code.

tglsfdc · tglsfdc · commit 5194024d72f3 · 2013-04-27T17:48:57.000-04:00
Move checking for unscannable matviews into ExecOpenScanRelation, which is
a better place for it first because the open relation is already available
(saving a relcache lookup cycle), and second because this eliminates the
problem of telling the difference between rangetable entries that will or
will not be scanned by the query.  In particular we can get rid of the
not-terribly-well-thought-out-or-implemented isResultRel field that the
initial matviews patch added to RangeTblEntry.

Also get rid of entirely unnecessary scannability check in the rewriter,
and a bogus decision about whether RefreshMatViewStmt requires a parse-time
snapshot.

catversion bump due to removal of a RangeTblEntry field, which changes
stored rules.
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
@@ -373,7 +373,6 @@ intorel_startup(DestReceiver *self, int operation, TupleDesc typeinfo)
 	rte->rtekind = RTE_RELATION;
 	rte->relid = intoRelationId;
 	rte->relkind = relkind;
-	rte->isResultRel = true;
 	rte->requiredPerms = ACL_INSERT;
 
 	for (attnum = 1; attnum <= intoRelationDesc->rd_att->natts; attnum++)
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
@@ -215,10 +215,8 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 	List       *rewritten;
 	PlannedStmt *plan;
 	QueryDesc  *queryDesc;
-	List	   *rtable;
-	RangeTblEntry	*initial_rte;
-	RangeTblEntry	*second_rte;
 
+	/* Rewrite, copying the given Query to make sure it's not changed */
 	rewritten = QueryRewrite((Query *) copyObject(query));
 
 	/* SELECT should never rewrite to more or less than one SELECT query */
@@ -229,26 +227,6 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 	/* Check for user-requested abort. */
 	CHECK_FOR_INTERRUPTS();
 
-	/*
-	 * Kludge here to allow refresh of a materialized view which is invalid
-	 * (that is, it was created or refreshed WITH NO DATA. We flag the first
-	 * two RangeTblEntry list elements, which were added to the front of the
-	 * rewritten Query to keep the rules system happy, with the isResultRel
-	 * flag to indicate that it is OK if they are flagged as invalid. See
-	 * UpdateRangeTableOfViewParse() for details.
-	 *
-	 * NOTE: The rewrite has switched the frist two RTEs, but they are still
-	 * in the first two positions. If that behavior changes, the asserts here
-	 * will fail.
-	 */
-	rtable = query->rtable;
-	initial_rte = ((RangeTblEntry *) linitial(rtable));
-	Assert(strcmp(initial_rte->alias->aliasname, "new"));
-	initial_rte->isResultRel = true;
-	second_rte = ((RangeTblEntry *) lsecond(rtable));
-	Assert(strcmp(second_rte->alias->aliasname, "old"));
-	second_rte->isResultRel = true;
-
 	/* Plan the query which will generate data for the refresh. */
 	plan = pg_plan_query(query, 0, NULL);
 
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
@@ -85,7 +85,6 @@ static char *ExecBuildSlotValueDescription(TupleTableSlot *slot,
 							  int maxfieldlen);
 static void EvalPlanQualStart(EPQState *epqstate, EState *parentestate,
 				  Plan *planTree);
-static bool RelationIdIsScannable(Oid relid);
 
 /* end of local decls */
 
@@ -494,63 +493,6 @@ ExecutorRewind(QueryDesc *queryDesc)
 }
 
 
-/*
- * ExecCheckRelationsScannable
- *		Check that relations which are to be accessed are in a scannable
- *		state.
- *
- * Currently the only relations which are not are materialized views which
- * have not been populated by their queries.
- */
-static void
-ExecCheckRelationsScannable(List *rangeTable)
-{
-	ListCell   *l;
-
-	foreach(l, rangeTable)
-	{
-		RangeTblEntry *rte = (RangeTblEntry *) lfirst(l);
-
-		if (rte->rtekind != RTE_RELATION)
-			continue;
-
-		if (rte->relkind != RELKIND_MATVIEW)
-			continue;
-
-		/* It is OK to target an unpopulated materialized for results. */
-		if (rte->isResultRel)
-			continue;
-
-		if (!RelationIdIsScannable(rte->relid))
-			ereport(ERROR,
-					(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-					 errmsg("materialized view \"%s\" has not been populated",
-							get_rel_name(rte->relid)),
-					 errhint("Use the REFRESH MATERIALIZED VIEW command.")));
-	}
-}
-
-/*
- * Tells whether a relation is scannable based on its OID.
- *
- * Currently only non-populated materialized views are not.  This is likely to
- * change to include other conditions.
- *
- * This should only be called while a lock is held on the relation.
- */
-static bool
-RelationIdIsScannable(Oid relid)
-{
-	Relation	relation;
-	bool		result;
-
-	relation = heap_open(relid, NoLock);
-	result = RelationIsScannable(relation);
-	heap_close(relation, NoLock);
-
-	return result;
-}
-
 /*
  * ExecCheckRTPerms
  *		Check access permissions for all relations listed in a range table.
@@ -941,20 +883,6 @@ InitPlan(QueryDesc *queryDesc, int eflags)
 	 */
 	planstate = ExecInitNode(plan, estate, eflags);
 
-	/*
-	 * Unless we are creating a view or are creating a materialized view WITH
-	 * NO DATA, ensure that all referenced relations are scannable.  The
-	 * omitted cases will be checked as SELECT statements in a different
-	 * phase, so checking again here would be wasteful and it would generate
-	 * errors on a materialized view referenced as a target.
-	 *
-	 * NB: This is being done after all relations are locked, files have been
-	 * opened, etc., to avoid duplicating that effort or creating deadlock
-	 * possibilities.
-	 */
-	if ((eflags & EXEC_FLAG_WITH_NO_DATA) == 0)
-		ExecCheckRelationsScannable(rangeTable);
-
 	/*
 	 * Get the tuple descriptor describing the type of tuples to return.
 	 */
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
@@ -798,8 +798,9 @@ ExecRelationIsTargetRelation(EState *estate, Index scanrelid)
  * ----------------------------------------------------------------
  */
 Relation
-ExecOpenScanRelation(EState *estate, Index scanrelid)
+ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
 {
+	Relation	rel;
 	Oid			reloid;
 	LOCKMODE	lockmode;
 
@@ -827,9 +828,24 @@ ExecOpenScanRelation(EState *estate, Index scanrelid)
 		}
 	}
 
-	/* OK, open the relation and acquire lock as needed */
+	/* Open the relation and acquire lock as needed */
 	reloid = getrelid(scanrelid, estate->es_range_table);
-	return heap_open(reloid, lockmode);
+	rel = heap_open(reloid, lockmode);
+
+	/*
+	 * Complain if we're attempting a scan of an unscannable relation, except
+	 * when the query won't actually be run.  This is a slightly klugy place
+	 * to do this, perhaps, but there is no better place.
+	 */
+	if ((eflags & (EXEC_FLAG_EXPLAIN_ONLY | EXEC_FLAG_WITH_NO_DATA)) == 0 &&
+		!RelationIsScannable(rel))
+		ereport(ERROR,
+				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+				 errmsg("materialized view \"%s\" has not been populated",
+						RelationGetRelationName(rel)),
+				 errhint("Use the REFRESH MATERIALIZED VIEW command.")));
+
+	return rel;
 }
 
 /* ----------------------------------------------------------------
diff --git a/src/backend/executor/nodeBitmapHeapscan.c b/src/backend/executor/nodeBitmapHeapscan.c
@@ -587,7 +587,7 @@ ExecInitBitmapHeapScan(BitmapHeapScan *node, EState *estate, int eflags)
 	/*
 	 * open the base relation and acquire appropriate lock on it.
 	 */
-	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
+	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
 
 	scanstate->ss.ss_currentRelation = currentRelation;
 
diff --git a/src/backend/executor/nodeForeignscan.c b/src/backend/executor/nodeForeignscan.c
@@ -143,7 +143,7 @@ ExecInitForeignScan(ForeignScan *node, EState *estate, int eflags)
 	/*
 	 * open the base relation and acquire appropriate lock on it.
 	 */
-	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
+	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
 	scanstate->ss.ss_currentRelation = currentRelation;
 
 	/*
diff --git a/src/backend/executor/nodeIndexonlyscan.c b/src/backend/executor/nodeIndexonlyscan.c
@@ -410,7 +410,7 @@ ExecInitIndexOnlyScan(IndexOnlyScan *node, EState *estate, int eflags)
 	/*
 	 * open the base relation and acquire appropriate lock on it.
 	 */
-	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
+	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
 
 	indexstate->ss.ss_currentRelation = currentRelation;
 	indexstate->ss.ss_currentScanDesc = NULL;	/* no heap scan here */
diff --git a/src/backend/executor/nodeIndexscan.c b/src/backend/executor/nodeIndexscan.c
@@ -511,7 +511,7 @@ ExecInitIndexScan(IndexScan *node, EState *estate, int eflags)
 	/*
 	 * open the base relation and acquire appropriate lock on it.
 	 */
-	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
+	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
 
 	indexstate->ss.ss_currentRelation = currentRelation;
 	indexstate->ss.ss_currentScanDesc = NULL;	/* no heap scan here */
diff --git a/src/backend/executor/nodeSeqscan.c b/src/backend/executor/nodeSeqscan.c
@@ -29,7 +29,7 @@
 #include "executor/nodeSeqscan.h"
 #include "utils/rel.h"
 
-static void InitScanRelation(SeqScanState *node, EState *estate);
+static void InitScanRelation(SeqScanState *node, EState *estate, int eflags);
 static TupleTableSlot *SeqNext(SeqScanState *node);
 
 /* ----------------------------------------------------------------
@@ -118,12 +118,11 @@ ExecSeqScan(SeqScanState *node)
 /* ----------------------------------------------------------------
  *		InitScanRelation
  *
- *		This does the initialization for scan relations and
- *		subplans of scans.
+ *		Set up to access the scan relation.
  * ----------------------------------------------------------------
  */
 static void
-InitScanRelation(SeqScanState *node, EState *estate)
+InitScanRelation(SeqScanState *node, EState *estate, int eflags)
 {
 	Relation	currentRelation;
 	HeapScanDesc currentScanDesc;
@@ -133,8 +132,10 @@ InitScanRelation(SeqScanState *node, EState *estate)
 	 * open that relation and acquire appropriate lock on it.
 	 */
 	currentRelation = ExecOpenScanRelation(estate,
-									 ((SeqScan *) node->ps.plan)->scanrelid);
+									 ((SeqScan *) node->ps.plan)->scanrelid,
+										   eflags);
 
+	/* initialize a heapscan */
 	currentScanDesc = heap_beginscan(currentRelation,
 									 estate->es_snapshot,
 									 0,
@@ -143,6 +144,7 @@ InitScanRelation(SeqScanState *node, EState *estate)
 	node->ss_currentRelation = currentRelation;
 	node->ss_currentScanDesc = currentScanDesc;
 
+	/* and report the scan tuple slot's rowtype */
 	ExecAssignScanType(node, RelationGetDescr(currentRelation));
 }
 
@@ -196,7 +198,7 @@ ExecInitSeqScan(SeqScan *node, EState *estate, int eflags)
 	/*
 	 * initialize scan relation
 	 */
-	InitScanRelation(scanstate, estate);
+	InitScanRelation(scanstate, estate, eflags);
 
 	scanstate->ps.ps_TupFromTlist = false;
 
diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c
@@ -531,7 +531,7 @@ ExecInitTidScan(TidScan *node, EState *estate, int eflags)
 	/*
 	 * open the base relation and acquire appropriate lock on it.
 	 */
-	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid);
+	currentRelation = ExecOpenScanRelation(estate, node->scan.scanrelid, eflags);
 
 	tidstate->ss.ss_currentRelation = currentRelation;
 	tidstate->ss.ss_currentScanDesc = NULL;		/* no heap scan here */
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
@@ -1972,7 +1972,6 @@ _copyRangeTblEntry(const RangeTblEntry *from)
 	COPY_SCALAR_FIELD(rtekind);
 	COPY_SCALAR_FIELD(relid);
 	COPY_SCALAR_FIELD(relkind);
-	COPY_SCALAR_FIELD(isResultRel);
 	COPY_NODE_FIELD(subquery);
 	COPY_SCALAR_FIELD(security_barrier);
 	COPY_SCALAR_FIELD(jointype);
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
@@ -2234,7 +2234,6 @@ _equalRangeTblEntry(const RangeTblEntry *a, const RangeTblEntry *b)
 	COMPARE_SCALAR_FIELD(rtekind);
 	COMPARE_SCALAR_FIELD(relid);
 	COMPARE_SCALAR_FIELD(relkind);
-	COMPARE_SCALAR_FIELD(isResultRel);
 	COMPARE_NODE_FIELD(subquery);
 	COMPARE_SCALAR_FIELD(security_barrier);
 	COMPARE_SCALAR_FIELD(jointype);
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
@@ -2353,7 +2353,6 @@ _outRangeTblEntry(StringInfo str, const RangeTblEntry *node)
 		case RTE_RELATION:
 			WRITE_OID_FIELD(relid);
 			WRITE_CHAR_FIELD(relkind);
-			WRITE_BOOL_FIELD(isResultRel);
 			break;
 		case RTE_SUBQUERY:
 			WRITE_NODE_FIELD(subquery);
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
@@ -1191,7 +1191,6 @@ _readRangeTblEntry(void)
 		case RTE_RELATION:
 			READ_OID_FIELD(relid);
 			READ_CHAR_FIELD(relkind);
-			READ_BOOL_FIELD(isResultRel);
 			break;
 		case RTE_SUBQUERY:
 			READ_NODE_FIELD(subquery);
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
@@ -325,11 +325,6 @@ analyze_requires_snapshot(Node *parseTree)
 			result = true;
 			break;
 
-		case T_RefreshMatViewStmt:
-			/* yes, because the SELECT from pg_rewrite must be analyzed */
-			result = true;
-			break;
-
 		default:
 			/* other utility statements don't have any real parse analysis */
 			result = false;
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
@@ -1579,6 +1579,19 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
 		if (rte->rtekind != RTE_RELATION)
 			continue;
 
+		/*
+		 * Always ignore RIR rules for materialized views referenced in
+		 * queries.  (This does not prevent refreshing MVs, since they aren't
+		 * referenced in their own query definitions.)
+		 *
+		 * Note: in the future we might want to allow MVs to be conditionally
+		 * expanded as if they were regular views, if they are not scannable.
+		 * In that case this test would need to be postponed till after we've
+		 * opened the rel, so that we could check its state.
+		 */
+		if (rte->relkind == RELKIND_MATVIEW)
+			continue;
+
 		/*
 		 * If the table is not referenced in the query, then we ignore it.
 		 * This prevents infinite expansion loop due to new rtable entries
@@ -1604,24 +1617,6 @@ fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
 		 */
 		rel = heap_open(rte->relid, NoLock);
 
-		/*
-		 * Ignore RIR rules for a materialized view, if it is scannable.
-		 *
-		 * NOTE: This is assuming that if an MV is scannable then we always
-		 * want to use the existing contents, and if it is not scannable we
-		 * cannot have gotten to this point unless it is being populated
-		 * (otherwise an error should be thrown).  It would be nice to have
-		 * some way to confirm that we're doing the right thing here, but rule
-		 * expansion doesn't give us a lot to work with, so we are trusting
-		 * earlier validations to throw error if needed.
-		 */
-		if (rel->rd_rel->relkind == RELKIND_MATVIEW &&
-			RelationIsScannable(rel))
-		{
-			heap_close(rel, NoLock);
-			continue;
-		}
-
 		/*
 		 * Collect the RIR rules that we must apply
 		 */
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
  */
 
 /*							yyyymmddN */
-#define CATALOG_VERSION_NO	201304151
+#define CATALOG_VERSION_NO	201304271
 
 #endif
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
@@ -341,7 +341,7 @@ extern void ExecAssignScanTypeFromOuterPlan(ScanState *scanstate);
 
 extern bool ExecRelationIsTargetRelation(EState *estate, Index scanrelid);
 
-extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid);
+extern Relation ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags);
 extern void ExecCloseScanRelation(Relation scanrel);
 
 extern void ExecOpenIndices(ResultRelInfo *resultRelInfo);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
@@ -713,7 +713,6 @@ typedef struct RangeTblEntry
 	 */
 	Oid			relid;			/* OID of the relation */
 	char		relkind;		/* relation kind (see pg_class.relkind) */
-	bool		isResultRel;	/* used in target of SELECT INTO or similar */
 
 	/*
 	 * Fields valid for a subquery RTE (else NULL):
@@ -2461,7 +2460,7 @@ typedef struct CreateTableAsStmt
 	NodeTag		type;
 	Node	   *query;			/* the query (see comments above) */
 	IntoClause *into;			/* destination table */
-	ObjectType	relkind;		/* type of object */
+	ObjectType	relkind;		/* OBJECT_TABLE or OBJECT_MATVIEW */
 	bool		is_select_into; /* it was written as SELECT INTO */
 } CreateTableAsStmt;
 
