8000 Ensure an index that uses a whole-row Var still depends on its table. · danielcode/postgres@2868b0c · GitHub
[go: up one dir, main page]
Skip to content

Commit 2868b0c

Browse files
tglsfdctglsfdc
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 c0486e9 commit 2868b0c

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
@@ -890,6 +890,12 @@ recordDependencyOnExpr(const ObjectAddress *depender,
890890
* range table. An additional frammish is that dependencies on that
891891
* relation (or its component columns) will be marked with 'self_behavior',
892892
* whereas 'behavior' is used for everything else.
893+
*
894+
* NOTE: the caller should ensure that a whole-table dependency on the
895+
* specified relation is created separately, if one is needed. In particular,
896+
* a whole-row Var "relation.*" will not cause this routine to emit any
897+
* dependency item. This is appropriate behavior for subexpressions of an
898+
* ordinary query, so other cases need to cope as necessary.
893899
*/
894900
void
895901
recordDependencyOnSingleRelExpr(const ObjectAddress *depender,
@@ -998,7 +1004,14 @@ find_expr_references_walker(Node *node,
9981004

9991005
/*
10001006
* A whole-row Var references no specific columns, so adds no new
1001-
* dependency.
1007+
* dependency. (We assume that there is a whole-table dependency
1008+
* arising from each underlying rangetable entry. While we could
1009+
* record such a dependency when finding a whole-row Var that
1010+
* references a relation directly, it's quite unclear how to extend
1011+
* that to whole-row Vars for JOINs, so it seems better to leave the
1012+
* responsibility with the range table. Note that this poses some
1013+
* risks for identifying dependencies of stand-alone expressions:
1014+
* whole-table references may need to be created separately.)
10021015
*/
10031016
if (var->varattno == InvalidAttrNumber)
10041017
return false;

‎src/backend/catalog/index.c

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
#include "executor/executor.h"
4040
#include "miscadmin.h"
4141
#include "optimizer/clauses.h"
42-
#include "optimizer/var.h"
4342
#include "parser/parse_expr.h"
4443
#include "storage/procarray.h"
4544
#include "storage/smgr.h"
@@ -689,16 +688,12 @@ index_create(Oid heapRelationId,
689688
}
690689

691690
/*
692-
* It's possible for an index to not depend on any columns of
693-
* the table at all, in which case we need to give it a dependency
694-
* on the table as a whole; else it won't get dropped when the
695-
* table is dropped. This edge case is not totally useless;
696-
* for example, a unique index on a constant expression can serve
697-
* to prevent a table from containing more than one row.
691+
* If there are no simply-referenced columns, give the index an
692+
* auto dependency on the whole table. In most cases, this will
693+
* be redundant, but it might not be if the index expressions and
694+
* predicate contain no Vars or only whole-row Vars.
698695
*/
699-
if (!have_simple_col &&
700-
!contain_vars_of_level((Node *) indexInfo->ii_Expressions, 0) &&
701-
!contain_vars_of_level((Node *) indexInfo->ii_Predicate, 0))
696+
if (!have_simple_col)
702697
{
703698
referenced.classId = RelationRelationId;
704699
referenced.objectId = heapRelationId;

‎src/backend/utils/adt/ruleutils.c

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,7 @@ static void get_opclass_name(Oid opclass, Oid actual_datatype,
215215
StringInfo buf);
216216
static Node *processIndirection(Node *node, deparse_context *context);
217217
static void printSubscripts(ArrayRef *aref, deparse_context *context);
218+
static char *get_relation_name(Oid relid);
218219
static char *generate_relation_name(Oid relid);
219220
static char *generate_function_name(Oid funcid, int nargs, Oid *argtypes);
220221
static char *generate_operator_name(Oid operid, Oid arg1, Oid arg2);
@@ -721,7 +722,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno, bool showTblSpc,
721722

722723
indexpr_item = list_head(indexprs);
723724

724-
context = deparse_context_for(get_rel_name(indrelid), indrelid);
725+
context = deparse_context_for(get_relation_name(indrelid), indrelid);
725726

726727
/*
727728
* Start the index definition. Note that the index's name should never be
@@ -1092,7 +1093,7 @@ pg_get_constraintdef_worker(Oid constraintId, bool fullCommand,
10921093
if (conForm->conrelid != InvalidOid)
10931094
{
10941095
/* relation constraint */
1095-
context = deparse_context_for(get_rel_name(conForm->conrelid),
1096+
context = deparse_context_for(get_relation_name(conForm->conrelid),
10961097
conForm->conrelid);
10971098
}
10981099
else
@@ -4329,7 +4330,7 @@ get_from_clause_item(Node *jtnode, Query *query, deparse_context *context)
43294330
gavealias = true;
43304331
}
43314332
else if (rte->rtekind == RTE_RELATION &&
4332-
strcmp(rte->eref->aliasname, get_rel_name(rte->relid)) != 0)
4333+
strcmp(rte->eref->aliasname, get_relation_name(rte->relid)) != 0)
43334334
{
43344335
/*
43354336
* Apparently the rel has been renamed since the rule was made.
@@ -4840,6 +4841,23 @@ quote_qualified_identifier(const char *namespace,
48404841
return buf.data;
48414842
}
48424843

4844+
/*
4845+
* get_relation_name
4846+
* Get the unqualified name of a relation specified by OID
4847+
*
4848+
* This differs from the underlying get_rel_name() function in that it will
4849+
* throw error instead of silently returning NULL if the OID is bad.
4850+
*/
4851+
static char *
4852+
get_relation_name(Oid relid)
4853+
{
4854+
char *relname = get_rel_name(relid);
4855+
4856+
if (!relname)
4857+
elog(ERROR, "cache lookup failed for relation %u", relid);
4858+
return relname;
4859+
}
4860+
48434861
/*
48444862
* generate_relation_name
48454863
* Compute the name to display for a relation specified by OID

0 commit comments

Comments
 (0)
0