Incidental cleanup of matviews code.

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
index 94a5fa755e30d82dffdbc82c7ce39fe40b75ea65..de65c4c78174d6c7daa72d49367dfc2bc789df61 100644 (file)
--- 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
index ac7719e40da2f62605cdefbcf77f8261f41d800f..da373045cc02fd62cade8b5b222e5de7627f3705 100644 (file)
--- 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
index 8d1d0aa927d30ffc0e18862d180a9b454e7ccaee..e1b280a065c2617e2e321c6737d751fc50062f37 100644 (file)
--- 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
index 11be62e91536cc55407058d213e9b1390c32efe9..cf7fb72ffcffe7f964acd1017f9197958f02fbc6 100644 (file)
--- 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
index c83f9722cf96f57e8353b5dcfac63099e4b5e2aa..d2b27213ffeb83f40273236ce57126ef115cc0fb 100644 (file)
--- 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
index 448fd6a912fae807075341dd2f8b20dae5386242..c0b3525e50d8139442636dcb73c9bbef790f80e8 100644 (file)
--- 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
index 7d01f8166e65fc1d4d3b2ddf92402ae7eef52083..2f30c55c54a46798c963e020be6ff0764edbe0e4 100644 (file)
--- 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
index c71a382af81df7c985f89d539e75999515e87312..f1062f19f43a1759950f00e6937899d60a7f0dfd 100644 (file)
--- 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
index c6f2eabedee6cd2b3673df63fd1f45fcd1e03642..c4edec0750b2b1ecbbdd3381039177b3b5e22c1b 100644 (file)
--- 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
index ced9c8b1b588127b3f8ed5515039582bdc47fe56..316a4ed0136077d184636a9d093dd6073c549f89 100644 (file)
--- 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
index 6bfbbf42135fb032019487f4af10091574d1f95b..b5b8d63cff79247ed1340b228fd5a10887a5a1bf 100644 (file)
--- 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
index 7b49f0afb956d3b08da6ccbe63fbe140f27401e8..7245fa32a0f0d7d7bb924eb8461d68a2a0d702ed 100644 (file)
--- 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
index bd47ddd0a267a7c36e42b01ba2bdbfee59c1cdb8..b2183f42137bd474925df18d96349ac99d5f0a80 100644 (file)
--- 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
index f275a31e3c2108bc3b70d5eed4a88283b15dec68..3a16e9db524e67af039b957cbaf53b9938bedf6a 100644 (file)
--- 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
index e5faf46a7a6947a4a50d74da7a071faff7c70444..fb28e471685c7462e8076556a78ef45b561f3d2e 100644 (file)
--- 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
index 4209b4c6d0f7243e5ea229fef40c48a955f9bc74..83f26e3f42eb66dc4fbc7df41294a3d7ba28f64e 100644 (file)
--- 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
index 83d8fd5e4bed93b4be595b2f97caa5b743be671b..42eb4de279431445ed7019f491d716548701226f 100644 (file)
--- 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
index aec6c7f7dfeae95a2fa9708a2adbc3ed2f573773..bc215d6c7d5d515bc6f8cbca3df144250c5cc980 100644 (file)
--- 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
index 6366e66e1368146e5a39849d8be591dcdf84cffb..49c2a3158eeb5925f604dde80c408b6250ef4586 100644 (file)
--- 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;