Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 9f376e1

Browse files
committed
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.
1 parent 35d5d96 commit 9f376e1

File tree

3 files changed

+40
-14
lines changed

3 files changed

+40
-14
lines changed

src/backend/catalog/dependency.c

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1238,6 +1238,12 @@ recordDependencyOnExpr(const ObjectAddress *depender,
12381238
* range table. An additional frammish is that dependencies on that
12391239
* relation (or its component columns) will be marked with 'self_behavior',
12401240
* whereas 'behavior' is used for everything else.
1241+
*
1242+
* NOTE: the caller should ensure that a whole-table dependency on the
1243+
* specified relation is created separately, if one is needed. In particular,
1244+
* a whole-row Var "relation.*" will not cause this routine to emit any
1245+
* dependency item. This is appropriate behavior for subexpressions of an
1246+
* ordinary query, so other cases need to cope as necessary.
12411247
*/
12421248
void
12431249
recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
@@ -1350,7 +1356,14 @@ find_expr_references_walker(Node *node,
13501356

13511357
/*
13521358
* A whole-row Var references no specific columns, so adds no new
1353-
* dependency.
1359+
* dependency. (We assume that there is a whole-table dependency
1360+
* arising from each underlying rangetable entry. While we could
1361+
* record such a dependency when finding a whole-row Var that
1362+
* references a relation directly, it's quite unclear how to extend
1363+
* that to whole-row Vars for JOINs, so it seems better to leave the
1364+
* responsibility with the range table. Note that this poses some
1365+
* risks for identifying dependencies of stand-alone expressions:
1366+
* whole-table references may need to be created separately.)
13541367
*/
13551368
if (var->varattno == InvalidAttrNumber)
13561369
return false;

src/backend/catalog/index.c

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,6 @@
5050
#include "nodes/makefuncs.h"
5151
#include "nodes/nodeFuncs.h"
5252
#include "optimizer/clauses.h"
53-
#include "optimizer/var.h"
5453
#include "parser/parser.h"
5554
#include "storage/bufmgr.h"
5655
#include "storage/lmgr.h"
@@ -858,16 +857,12 @@ index_create(Oid heapRelationId,
858857
}
859858

860859
/*
861-
* It's possible for an index to not depend on any columns of the
862-
* table at all, in which case we need to give it a dependency on
863-
* the table as a whole; else it won't get dropped when the table
864-
* is dropped. This edge case is not totally useless; for
865-
* example, a unique index on a constant expression can serve to
866-
* prevent a table from containing more than one row.
860+
* If there are no simply-referenced columns, give the index an
861+
* auto dependency on the whole table. In most cases, this will
862+
* be redundant, but it might not be if the index expressions and
863+
* predicate contain no Vars or only whole-row Vars.
867864
*/
868-
if (!have_simple_col &&
869-
!contain_vars_of_level((Node *) indexInfo->ii_Expressions, 0) &&
870-
!contain_vars_of_level((Node *) indexInfo->ii_Predicate, 0))
865+
if (!have_simple_col)
871866
{
872867
referenced.classId = RelationRelationId;
873868
referenced.objectId = heapRelationId;

src/backend/utils/adt/ruleutils.c

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,6 +240,7 @@ static void get_opclass_name(Oid opclass, Oid actual_datatype,
240240
static Node *processIndirection(Node *node, deparse_context *context,
241241
bool printit);
242242
static void printSubscripts(ArrayRef *aref, deparse_context *context);
243+
static char *get_relation_name(Oid relid);
243244
static char *generate_relation_name(Oid relid, List *namespaces);
244245
static char *generate_function_name(Oid funcid, int nargs, List *argnames,
245246
Oid *argtypes, bool *is_variadic);
@@ -857,7 +858,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
857858

858859
indexpr_item = list_head(indexprs);
859860

860-
context = deparse_context_for(get_rel_name(indrelid), indrelid);
861+
context = deparse_context_for(get_relation_name(indrelid), indrelid);
861862

862863
/*
863864
* 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,
12611262
if (conForm->conrelid != InvalidOid)
12621263
{
12631264
/* relation constraint */
1264-
context = deparse_context_for(get_rel_name(conForm->conrelid),
1265+
context = deparse_context_for(get_relation_name(conForm->conrelid),
12651266
conForm->conrelid);
12661267
}
12671268
else
@@ -6314,7 +6315,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
63146315
gavealias = true;
63156316
}
63166317
else if (rte->rtekind == RTE_RELATION &&
6317-
strcmp(rte->eref->aliasname, get_rel_name(rte->relid)) != 0)
6318+
strcmp(rte->eref->aliasname, get_relation_name(rte->relid)) != 0)
63186319
{
63196320
/*
63206321
* Apparently the rel has been renamed since the rule was made.
@@ -6817,6 +6818,23 @@ quote_qualified_identifier(const char *qualifier,
68176818
return buf.data;
68186819
}
68196820

6821+
/*
6822+
* get_relation_name
6823+
* Get the unqualified name of a relation specified by OID
6824+
*
6825+
* This differs from the underlying get_rel_name() function in that it will
6826+
* throw error instead of silently returning NULL if the OID is bad.
6827+
*/
6828+
static char *
6829+
get_relation_name(Oid relid)
6830+
{
6831+
char *relname = get_rel_name(relid);
6832+
6833+
if (!relname)
6834+
elog(ERROR, "cache lookup failed for relation %u", relid);
6835+
return relname;
6836+
}
6837+
68206838
/*
68216839
* generate_relation_name
68226840
* Compute the name to display for a relation specified by OID

0 commit comments

Comments
 (0)