Ensure an index that uses a whole-row Var still depends on its table.

We failed to record any dependency on the underlying table for an index
declared like "create index i on t (foo(t.*))".  This would create trouble
if the table were dropped without previously dropping the index.  To fix,
simplify some overly-cute code in index_create(), accepting the possibility
that sometimes the whole-table dependency will be redundant.  Also document
this hazard in dependency.c.  Per report from Kevin Grittner.

In passing, prevent a core dump in pg_get_indexdef() if the index's table
can't be found.  I came across this while experimenting with Kevin's
example.  Not sure it's a real issue when the catalogs aren't corrupt, but
might as well be cautious.

Back-patch to all supported versions.

diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c
index 18e07eb956a4f6eb4a4fcfd6bb8f0523996a984d..bb14a438b0a3beaf3bd50bfbb45976f77b85273c 100644 (file)
--- a/src/backend/catalog/dependency.c
+++ b/src/backend/catalog/dependency.c
@@ -1238,6 +1238,12 @@ recordDependencyOnExpr(const ObjectAddress *depender,
  * range table.  An additional frammish is that dependencies on that
  * relation (or its component columns) will be marked with 'self_behavior',
  * whereas 'behavior' is used for everything else.
+ *
+ * NOTE: the caller should ensure that a whole-table dependency on the
+ * specified relation is created separately, if one is needed.  In particular,
+ * a whole-row Var "relation.*" will not cause this routine to emit any
+ * dependency item.  This is appropriate behavior for subexpressions of an
+ * ordinary query, so other cases need to cope as necessary.
  */
 void
 recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
@@ -1350,7 +1356,14 @@ find_expr_references_walker(Node *node,
 
                /*
                 * A whole-row Var references no specific columns, so adds no new
-                * dependency.
+                * dependency.  (We assume that there is a whole-table dependency
+                * arising from each underlying rangetable entry.  While we could
+                * record such a dependency when finding a whole-row Var that
+                * references a relation directly, it's quite unclear how to extend
+                * that to whole-row Vars for JOINs, so it seems better to leave the
+                * responsibility with the range table.  Note that this poses some
+                * risks for identifying dependencies of stand-alone expressions:
+                * whole-table references may need to be created separately.)
                 */
                if (var->varattno == InvalidAttrNumber)
                        return false;

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index b82b9b66fecadb7f70aa32fd86e9bba51aae9193..b437c9976a1ea9dc17bda274cdc5b2b73d0e23b1 100644 (file)
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -50,7 +50,6 @@
 #include "nodes/makefuncs.h"
 #include "nodes/nodeFuncs.h"
 #include "optimizer/clauses.h"
-#include "optimizer/var.h"
 #include "parser/parser.h"
 #include "storage/bufmgr.h"
 #include "storage/lmgr.h"
@@ -858,16 +857,12 @@ index_create(Oid heapRelationId,
                        }
 
                        /*
-                        * It's possible for an index to not depend on any columns of the
-                        * table at all, in which case we need to give it a dependency on
-                        * the table as a whole; else it won't get dropped when the table
-                        * is dropped.  This edge case is not totally useless; for
-                        * example, a unique index on a constant expression can serve to
-                        * prevent a table from containing more than one row.
+                        * If there are no simply-referenced columns, give the index an
+                        * auto dependency on the whole table.  In most cases, this will
+                        * be redundant, but it might not be if the index expressions and
+                        * predicate contain no Vars or only whole-row Vars.
                         */
-                       if (!have_simple_col &&
-                        !contain_vars_of_level((Node *) indexInfo->ii_Expressions, 0) &&
-                               !contain_vars_of_level((Node *) indexInfo->ii_Predicate, 0))
+                       if (!have_simple_col)
                        {
                                referenced.classId = RelationRelationId;
                                referenced.objectId = heapRelationId;

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index a5cc48b47cc7fcbef32b3248906a59983a63a536..dc17685563bad4f1141b44becbf05f7b204c4b34 100644 (file)
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -240,6 +240,7 @@ static void get_opclass_name(Oid opclass, Oid actual_datatype,
 static Node *processIndirection(Node *node, deparse_context *context,
                                   bool printit);
 static void printSubscripts(ArrayRef *aref, deparse_context *context);
+static char *get_relation_name(Oid relid);
 static char *generate_relation_name(Oid relid, List *namespaces);
 static char *generate_function_name(Oid funcid, int nargs, List *argnames,
                                           Oid *argtypes, bool *is_variadic);
@@ -857,7 +858,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
 
        indexpr_item = list_head(indexprs);
 
-       context = deparse_context_for(get_rel_name(indrelid), indrelid);
+       context = deparse_context_for(get_relation_name(indrelid), indrelid);
 
        /*
         * Start the index definition.  Note that the index's name should never be
@@ -1261,7 +1262,7 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
                                if (conForm->conrelid != InvalidOid)
                                {
                                        /* relation constraint */
-                                       context = deparse_context_for(get_rel_name(conForm->conrelid),
+                                       context = deparse_context_for(get_relation_name(conForm->conrelid),
                                                                                                  conForm->conrelid);
                                }
                                else
@@ -6314,7 +6315,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
                        gavealias = true;
                }
                else if (rte->rtekind == RTE_RELATION &&
-                                strcmp(rte->eref->aliasname, get_rel_name(rte->relid)) != 0)
+                                strcmp(rte->eref->aliasname, get_relation_name(rte->relid)) != 0)
                {
                        /*
                         * Apparently the rel has been renamed since the rule was made.
@@ -6817,6 +6818,23 @@ quote_qualified_identifier(const char *qualifier,
        return buf.data;
 }
 
+/*
+ * get_relation_name
+ *             Get the unqualified name of a relation specified by OID
+ *
+ * This differs from the underlying get_rel_name() function in that it will
+ * throw error instead of silently returning NULL if the OID is bad.
+ */
+static char *
+get_relation_name(Oid relid)
+{
+       char       *relname = get_rel_name(relid);
+
+       if (!relname)
+               elog(ERROR, "cache lookup failed for relation %u", relid);
+       return relname;
+}
+
 /*
  * generate_relation_name
  *             Compute the name to display for a relation specified by OID