From ac19ffb7f36b847f04692ab7c4849a17c143e56d Mon Sep 17 00:00:00 2001 From: Matthias van de Meent Date: Wed, 29 Jul 2026 19:45:33 +0200 Subject: [PATCH v5] Track whole-row Var dependencies in pg_depend This enables correct index and constraint validation when a table changes shape through DDL; and with it when the whole-row Var's value changes. Without this precise tracking, we'd have to scan all constraints and indexes for invalidations every time a relevant DDL is applied, which would make this prohibitively expensive. Tests were added to make sure these changes correctly rebuild indexes, as well as to test the new "whole row" object types. --- src/backend/catalog/aclchk.c | 2 + src/backend/catalog/dependency.c | 42 ++-- src/backend/catalog/objectaddress.c | 96 +++++++- src/backend/commands/dropcmds.c | 1 + src/backend/commands/event_trigger.c | 2 + src/backend/commands/seclabel.c | 1 + src/backend/commands/tablecmds.c | 228 ++++--------------- src/include/catalog/dependency.h | 9 + src/include/nodes/parsenodes.h | 1 + src/test/regress/expected/indexing.out | 14 ++ src/test/regress/expected/object_address.out | 26 ++- src/test/regress/sql/indexing.sql | 9 + src/test/regress/sql/object_address.sql | 7 +- 13 files changed, 239 insertions(+), 199 deletions(-) diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index e67358f3858..8a77417449d 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -2775,6 +2775,7 @@ aclcheck_error(AclResult aclerr, ObjectType objtype, case OBJECT_TSPARSER: case OBJECT_TSTEMPLATE: case OBJECT_USER_MAPPING: + case OBJECT_WHOLE_ROW: elog(ERROR, "unsupported object type: %d", objtype); } @@ -2913,6 +2914,7 @@ aclcheck_error(AclResult aclerr, ObjectType objtype, case OBJECT_TSPARSER: case OBJECT_TSTEMPLATE: case OBJECT_USER_MAPPING: + case OBJECT_WHOLE_ROW: elog(ERROR, "unsupported object type: %d", objtype); } diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index e0fe2a477b0..dff354fa192 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -1638,10 +1638,12 @@ collectDependenciesOfExpr(ObjectAddresses *addrs, * to depend on the table not vice versa. * * 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. + * specified relation is created separately, if one is needed. + * In particular, a whole-row Var "relation.*" will cause this routine + * to emit a dependency on the WholeRowAttrNumber subobject, not the + * whole object (subobj 0). + * This is appropriate behavior for expressions in ordinary indexes, so + * other cases need to cope as necessary. */ void recordDependencyOnSingleRelExpr(const ObjectAddress *depender, @@ -1830,6 +1832,7 @@ find_expr_references_walker(Node *node, if (IsA(node, Var)) { Var *var = (Var *) node; + AttrNumber attno; List *rtable; RangeTblEntry *rte; @@ -1842,29 +1845,29 @@ find_expr_references_walker(Node *node, rte = rt_fetch(var->varno, rtable); /* - * A whole-row Var references no specific columns, so adds no new - * 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.) + * A whole-row Var references no specific columns. We could assume + * that there is a whole-table dependency arising from each + * underlying rangetable entry, and in doing so ignore the + * whole-row var, but doing so would make it difficult to find the + * objects that contain whole-row Vars which need to be checked, + * invalidated, or rebuilt when the referenced table changes shape. */ if (var->varattno == InvalidAttrNumber) - return false; + attno = WholeRowAttrNumber; + else + attno = var->varattno; + if (rte->rtekind == RTE_RELATION) { /* If it's a plain relation, reference this column */ - add_object_address(RelationRelationId, rte->relid, var->varattno, + add_object_address(RelationRelationId, rte->relid, attno, context->addrs); } else if (rte->rtekind == RTE_FUNCTION) { /* Might need to add a dependency on a composite type's column */ /* (done out of line, because it's a bit bulky) */ - process_function_rte_ref(rte, var->varattno, context); + process_function_rte_ref(rte, attno, context); } /* @@ -2522,6 +2525,13 @@ process_function_rte_ref(RangeTblEntry *rte, AttrNumber attnum, if (rte->funcordinality && attnum == atts_done + 1) return; + /* + * Signatures of functions and procedures are immutable, so no need to + * track its dependents. + */ + if (attnum == WholeRowAttrNumber) + return; + /* this probably can't happen ... */ ereport(ERROR, (errcode(ERRCODE_UNDEFINED_COLUMN), diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index b08e076e65f..46fac3dd7af 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -682,27 +682,51 @@ static const struct object_type_map { "table column", OBJECT_COLUMN }, + { + "table whole row", OBJECT_WHOLE_ROW + }, /* unmapped */ { "index column", -1 }, /* unmapped */ + { + "index whole row", -1 + }, /* unmapped */ { "sequence column", -1 }, /* unmapped */ + { + "sequence whole row", -1 + }, /* unmapped */ { "toast table column", -1 }, /* unmapped */ + { + "toast table whole row", -1 + }, /* unmapped */ { "view column", -1 }, /* unmapped */ + { + "view whole row", -1 + }, /* unmapped */ { "materialized view column", -1 }, /* unmapped */ + { + "materialized view whole row", -1 + }, /* unmapped */ { "composite type column", -1 }, /* unmapped */ + { + "composite type whole row", -1 + }, /* unmapped */ { "foreign table column", OBJECT_COLUMN }, + { + "foreign table whole row", OBJECT_WHOLE_ROW + }, { "aggregate", OBJECT_AGGREGATE }, @@ -851,6 +875,9 @@ static ObjectAddress get_object_address_relobject(ObjectType objtype, static ObjectAddress get_object_address_attribute(ObjectType objtype, List *object, Relation *relp, LOCKMODE lockmode, bool missing_ok); +static ObjectAddress get_object_address_wholerow(ObjectType objtype, + List *object, Relation *relp, + LOCKMODE lockmode, bool missing_ok); static ObjectAddress get_object_address_attrdef(ObjectType objtype, List *object, Relation *relp, LOCKMODE lockmode, bool missing_ok); @@ -961,6 +988,12 @@ get_object_address(ObjectType objtype, Node *object, &relation, lockmode, missing_ok); break; + case OBJECT_WHOLE_ROW: + address = + get_object_address_wholerow(objtype, castNode(List, object), + &relation, lockmode, + missing_ok); + break; case OBJECT_DEFAULT: address = get_object_address_attrdef(objtype, castNode(List, object), @@ -1397,6 +1430,12 @@ get_relation_by_qualified_name(ObjectType objtype, List *object, errmsg("\"%s\" is not a foreign table", RelationGetRelationName(relation)))); break; + case OBJECT_WHOLE_ROW: + /* + * Whole row objects don't carry their underlying type, so + * we assume it's OK + */ + break; default: elog(ERROR, "unrecognized object type: %d", (int) objtype); break; @@ -1543,6 +1582,37 @@ get_object_address_attribute(ObjectType objtype, List *object, return address; } +/* + * Locate a relation by qualified name. + */ +static ObjectAddress +get_object_address_wholerow(ObjectType objtype, List *object, + Relation *relp, LOCKMODE lockmode, + bool missing_ok) +{ + Relation relation; + ObjectAddress address; + + address.classId = RelationRelationId; + address.objectId = InvalidOid; + address.objectSubId = 0; + + relation = relation_openrv_extended(makeRangeVarFromNameList(object), + lockmode, missing_ok); + + if (!relation) + return address; + + Assert(objtype == OBJECT_WHOLE_ROW); + + /* Done. */ + address.objectId = RelationGetRelid(relation); + address.objectSubId = WholeRowAttrNumber; + *relp = relation; + + return address; +} + /* * Find the ObjectAddress for an attribute's default value. */ @@ -2296,6 +2366,7 @@ pg_get_object_address(PG_FUNCTION_ARGS) case OBJECT_TABCONSTRAINT: case OBJECT_OPCLASS: case OBJECT_OPFAMILY: + case OBJECT_WHOLE_ROW: objnode = (Node *) name; break; case OBJECT_ACCESS_METHOD: @@ -2557,6 +2628,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address, case OBJECT_PUBLICATION_NAMESPACE: case OBJECT_PUBLICATION_REL: case OBJECT_USER_MAPPING: + case OBJECT_WHOLE_ROW: /* These are currently not supported or don't make sense here. */ elog(ERROR, "unsupported object type: %d", (int) objtype); break; @@ -2915,6 +2987,16 @@ getObjectDescription(const ObjectAddress *object, bool missing_ok) case RelationRelationId: if (object->objectSubId == 0) getRelationDescription(&buffer, object->objectId, missing_ok); + else if (object->objectSubId == WholeRowAttrNumber) + { + StringInfoData rel; + + initStringInfo(&rel); + getRelationDescription(&rel, object->objectId, missing_ok); + /* translator: %s is, e.g., "table %s" */ + appendStringInfo(&buffer, _("whole row of %s"), rel.data); + pfree(rel.data); + } else { /* column, not whole relation */ @@ -4732,8 +4814,14 @@ getRelationTypeDescription(StringInfo buffer, Oid relid, int32 objectSubId, break; } - if (objectSubId != 0) + if (objectSubId == WholeRowAttrNumber) + { + appendStringInfoString(buffer, " whole row"); + } + else if (objectSubId != 0) + { appendStringInfoString(buffer, " column"); + } ReleaseSysCache(relTup); } @@ -4862,7 +4950,7 @@ getObjectIdentityParts(const ObjectAddress *object, * Check for the attribute first, so as if it is missing we * can skip the entire relation description. */ - if (object->objectSubId != 0) + if (object->objectSubId != 0 && object->objectSubId != WholeRowAttrNumber) { attr = get_attname(object->objectId, object->objectSubId, @@ -4884,6 +4972,10 @@ getObjectIdentityParts(const ObjectAddress *object, if (objname) *objname = lappend(*objname, attr); } + else if (object->objectSubId == WholeRowAttrNumber) + { + appendStringInfo(&buffer, ".*"); + } } break; diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c index 92526012d2a..c94e775f2e7 100644 --- a/src/backend/commands/dropcmds.c +++ b/src/backend/commands/dropcmds.c @@ -508,6 +508,7 @@ does_not_exist_skipping(ObjectType objtype, Node *object) case OBJECT_PUBLICATION_REL: case OBJECT_TABCONSTRAINT: case OBJECT_USER_MAPPING: + case OBJECT_WHOLE_ROW: /* These are currently not used or needed. */ elog(ERROR, "unsupported object type: %d", (int) objtype); break; diff --git a/src/backend/commands/event_trigger.c b/src/backend/commands/event_trigger.c index d868c7f42c3..bab50a59845 100644 --- a/src/backend/commands/event_trigger.c +++ b/src/backend/commands/event_trigger.c @@ -2331,6 +2331,7 @@ stringify_grant_objtype(ObjectType objtype) case OBJECT_TSTEMPLATE: case OBJECT_USER_MAPPING: case OBJECT_VIEW: + case OBJECT_WHOLE_ROW: elog(ERROR, "unsupported object type: %d", (int) objtype); } @@ -2415,6 +2416,7 @@ stringify_adefprivs_objtype(ObjectType objtype) case OBJECT_TSTEMPLATE: case OBJECT_USER_MAPPING: case OBJECT_VIEW: + case OBJECT_WHOLE_ROW: elog(ERROR, "unsupported object type: %d", (int) objtype); } diff --git a/src/backend/commands/seclabel.c b/src/backend/commands/seclabel.c index 5b80396723c..d298816999c 100644 --- a/src/backend/commands/seclabel.c +++ b/src/backend/commands/seclabel.c @@ -92,6 +92,7 @@ SecLabelSupportsObjectType(ObjectType objtype) case OBJECT_TSPARSER: case OBJECT_TSTEMPLATE: case OBJECT_USER_MAPPING: + case OBJECT_WHOLE_ROW: return false; /* diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 8dc70bfa0f1..38418e182dd 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -550,7 +550,8 @@ static ObjectAddress ATExecSetStorage(Relation rel, const char *colName, static void ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing, AlterTableCmd *cmd, LOCKMODE lockmode, AlterTableUtilityContext *context); -static ObjectAddress ATExecDropColumn(List **wqueue, Relation rel, const char *colName, +static ObjectAddress ATExecDropColumn(List **wqueue, AlteredTableInfo *tab, + Relation rel, const char *colName, DropBehavior behavior, bool recurse, bool recursing, bool missing_ok, LOCKMODE lockmode, @@ -5399,7 +5400,8 @@ ATRewriteCatalogs(List **wqueue, LOCKMODE lockmode, * (this is not done in ATExecAlterColumnType since it should be * done only once if multiple columns of a table are altered). */ - if (pass == AT_PASS_ALTER_TYPE || pass == AT_PASS_SET_EXPRESSION) + if (pass == AT_PASS_ADD_COL || pass == AT_PASS_DROP || + pass == AT_PASS_ALTER_TYPE || pass == AT_PASS_SET_EXPRESSION) ATPostAlterTypeCleanup(wqueue, tab, lockmode); if (tab->rel) @@ -5498,7 +5500,7 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab, lockmode); break; case AT_DropColumn: /* DROP COLUMN */ - address = ATExecDropColumn(wqueue, rel, cmd->name, + address = ATExecDropColumn(wqueue, tab, rel, cmd->name, cmd->behavior, cmd->recurse, false, cmd->missing_ok, lockmode, NULL); @@ -7434,6 +7436,12 @@ ATExecAddColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, (errcode(ERRCODE_INVALID_TABLE_DEFINITION), errmsg("cannot recursively add identity column to table that has child tables"))); + /* + * Remember we need to invalidate whole-row dependents, to make sure they + * correctly handle the new column. + */ + RememberWholeRowDependentForRebuilding(tab, AT_AddColumn, rel); + pgclass = table_open(RelationRelationId, RowExclusiveLock); reltup = SearchSysCacheCopy1(RELOID, ObjectIdGetDatum(myrelid)); @@ -9352,11 +9360,10 @@ ATPrepDropColumn(List **wqueue, Relation rel, bool recurse, bool recursing, * checked recursively. */ static ObjectAddress -ATExecDropColumn(List **wqueue, Relation rel, const char *colName, - DropBehavior behavior, - bool recurse, bool recursing, - bool missing_ok, LOCKMODE lockmode, - ObjectAddresses *addrs) +ATExecDropColumn(List **wqueue, AlteredTableInfo *tab, Relation rel, + const char *colName, DropBehavior behavior, + bool recurse, bool recursing, bool missing_ok, + LOCKMODE lockmode, ObjectAddresses *addrs) { HeapTuple tuple; Form_pg_attribute targetatt; @@ -9436,6 +9443,9 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, ReleaseSysCache(tuple); + /* Invalidate whole-row */ + RememberWholeRowDependentForRebuilding(tab, AT_DropColumn, rel); + /* * Propagate to children as appropriate. Unlike most other ALTER * routines, we have to do this one level of recursion at a time; we can't @@ -9490,9 +9500,9 @@ ATExecDropColumn(List **wqueue, Relation rel, const char *colName, if (childatt->attinhcount == 1 && !childatt->attislocal) { /* Time to delete this child column, too */ - ATExecDropColumn(wqueue, childrel, colName, - behavior, true, true, - false, lockmode, addrs); + ATExecDropColumn(wqueue, tab, childrel, + colName, behavior, true, + true, false, lockmode, addrs); } else { @@ -15316,6 +15326,13 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, */ RememberAllDependentForRebuilding(tab, AT_AlterColumnType, rel, attnum, colName); + /* + * Find whole-row referenced objects that depend on the column + * (constraints, indexes, etc.), and record enough information to let us + * recreate the objects. + */ + RememberWholeRowDependentForRebuilding(tab, AT_AlterColumnType, rel); + /* * Now scan for dependencies of this column on other things. The only * things we should find are the dependency on the column datatype and @@ -15511,8 +15528,9 @@ ATExecAlterColumnType(AlteredTableInfo *tab, Relation rel, } /* - * Subroutine for ATExecAlterColumnType and ATExecSetExpression: Find everything - * that depends on the column (constraints, indexes, etc), and record enough + * Subroutine for ATExecAlterColumnType, ATExecSetExpression, ATExecAddColumn, + * and ATExecDropColumn: Find everything that depends on the column or + * whole-row reference (constraints, indexes, etc), and record enough * information to let us recreate the objects. */ static void @@ -15524,7 +15542,9 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, SysScanDesc scan; HeapTuple depTup; - Assert(subtype == AT_AlterColumnType || subtype == AT_SetExpression); + Assert(subtype == AT_AlterColumnType || subtype == AT_SetExpression || + (attnum == WholeRowAttrNumber && (subtype == AT_AddColumn || + subtype == AT_DropColumn))); depRel = table_open(DependRelationId, RowExclusiveLock); @@ -15598,8 +15618,10 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, * * This is only a problem for AT_AlterColumnType, not * AT_SetExpression. + * + * Whole Row expressions are ignored. */ - if (subtype == AT_AlterColumnType) + if (subtype == AT_AlterColumnType && attnum != WholeRowAttrNumber) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used by a function or procedure"), @@ -15614,7 +15636,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, * View/rule bodies have pretty much the same issues as * function bodies. FIXME someday. */ - if (subtype == AT_AlterColumnType) + if (subtype == AT_AlterColumnType && attnum != WholeRowAttrNumber) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used by a view or rule"), @@ -15634,7 +15656,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, * issues as above. Since we can't easily tell which case * applies, we punt for both. FIXME someday. */ - if (subtype == AT_AlterColumnType) + if (subtype == AT_AlterColumnType && attnum != WholeRowAttrNumber) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used in a trigger definition"), @@ -15653,7 +15675,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, * easy enough to remove and recreate the policy; still, FIXME * someday. */ - if (subtype == AT_AlterColumnType) + if (subtype == AT_AlterColumnType && attnum != WholeRowAttrNumber) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used in a policy definition"), @@ -15684,7 +15706,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, * by SQL standard, so just punt for now. It might be * doable with some thinking and effort. */ - if (subtype == AT_AlterColumnType) + if (subtype == AT_AlterColumnType && attnum != WholeRowAttrNumber) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used by a generated column"), @@ -15712,7 +15734,7 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, * Column reference in a PUBLICATION ... FOR TABLE ... WHERE * clause. Same issues as above. FIXME someday. */ - if (subtype == AT_AlterColumnType) + if (subtype == AT_AlterColumnType && attnum != WholeRowAttrNumber) ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), errmsg("cannot alter type of a column used by a publication WHERE clause"), @@ -15749,160 +15771,8 @@ RememberAllDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, static void RememberWholeRowDependentForRebuilding(AlteredTableInfo *tab, AlterTableType subtype, Relation rel) { - ScanKeyData skey; - Relation pg_constraint; - Relation pg_index; - SysScanDesc conscan; - SysScanDesc indscan; - HeapTuple constrTuple; - HeapTuple indexTuple; - bool isnull; - - Assert(subtype == AT_SetExpression); - - /* - * Check CHECK constraints with whole-row references first. - */ - if (RelationGetDescr(rel)->constr && - RelationGetDescr(rel)->constr->num_check > 0) - { - pg_constraint = table_open(ConstraintRelationId, AccessShareLock); - - ScanKeyInit(&skey, - Anum_pg_constraint_conrelid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationGetRelid(rel))); - - conscan = systable_beginscan(pg_constraint, - ConstraintRelidTypidNameIndexId, - true, - NULL, - 1, - &skey); - - while (HeapTupleIsValid(constrTuple = systable_getnext(conscan))) - { - Form_pg_constraint conform = (Form_pg_constraint) GETSTRUCT(constrTuple); - Datum exprDatum; - - if (conform->contype != CONSTRAINT_CHECK) - continue; - - exprDatum = heap_getattr(constrTuple, - Anum_pg_constraint_conbin, - RelationGetDescr(pg_constraint), - &isnull); - if (isnull) - elog(ERROR, "null conbin for relation \"%s\"", - RelationGetRelationName(rel)); - else - { - char *exprString; - Node *expr; - Bitmapset *expr_attrs = NULL; - - exprString = TextDatumGetCString(exprDatum); - expr = stringToNode(exprString); - pfree(exprString); - - /* Find all attributes referenced */ - pull_varattnos(expr, 1, &expr_attrs); - - /* - * If the CHECK constraint contains whole-row reference then - * remember it. - */ - if (bms_is_member(InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber, expr_attrs)) - { - RememberConstraintForRebuilding(conform->oid, tab); - } - } - } - systable_endscan(conscan); - table_close(pg_constraint, AccessShareLock); - } - - /* - * Now check indexes with whole-row references. Prepare to scan pg_index - * for entries having indrelid matching this relation. - */ - ScanKeyInit(&skey, - Anum_pg_index_indrelid, - BTEqualStrategyNumber, F_OIDEQ, - ObjectIdGetDatum(RelationGetRelid(rel))); - - pg_index = table_open(IndexRelationId, AccessShareLock); - - indscan = systable_beginscan(pg_index, - IndexIndrelidIndexId, - true, - NULL, - 1, - &skey); - - while (HeapTupleIsValid(indexTuple = systable_getnext(indscan))) - { - Form_pg_index index = (Form_pg_index) GETSTRUCT(indexTuple); - Datum exprDatum; - - exprDatum = heap_getattr(indexTuple, - Anum_pg_index_indexprs, - RelationGetDescr(pg_index), - &isnull); - if (!isnull) - { - char *exprString; - Node *expr; - Bitmapset *expr_attrs = NULL; - - exprString = TextDatumGetCString(exprDatum); - expr = stringToNode(exprString); - pfree(exprString); - - /* Find all attributes referenced */ - pull_varattnos(expr, 1, &expr_attrs); - - /* - * If the index expression contains a whole-row reference then - * remember it. - */ - if (bms_is_member(InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber, expr_attrs)) - { - RememberIndexForRebuilding(index->indexrelid, tab); - continue; - } - } - - exprDatum = heap_getattr(indexTuple, - Anum_pg_index_indpred, - RelationGetDescr(pg_index), - &isnull); - if (!isnull) - { - char *exprString; - Node *expr; - Bitmapset *expr_attrs = NULL; - - exprString = TextDatumGetCString(exprDatum); - expr = stringToNode(exprString); - pfree(exprString); - - /* Find all attributes referenced */ - pull_varattnos(expr, 1, &expr_attrs); - - /* - * If the index predicate expression contains a whole-row - * reference then remember it. - */ - if (bms_is_member(InvalidAttrNumber - FirstLowInvalidHeapAttributeNumber, expr_attrs)) - { - RememberIndexForRebuilding(index->indexrelid, tab); - } - } - } - - systable_endscan(indscan); - table_close(pg_index, AccessShareLock); + RememberAllDependentForRebuilding(tab, subtype, rel, + WholeRowAttrNumber, "*"); } /* @@ -16084,11 +15954,11 @@ RememberStatisticsForRebuilding(Oid stxoid, AlteredTableInfo *tab) } /* - * Cleanup after we've finished all the ALTER TYPE or SET EXPRESSION - * operations for a particular relation. We have to drop and recreate all the - * indexes and constraints that depend on the altered columns. We do the - * actual dropping here, but re-creation is managed by adding work queue - * entries to do those steps later. + * Cleanup after we've finished all the ADD COLUMN, DROP COLUMN, ALTER TYPE, + * or SET EXPRESSION operations for a particular relation. We have to drop + * and recreate all the indexes and constraints that depend on the altered + * columns. We do the actual dropping here, but re-creation is managed by + * adding work queue entries to do those steps later. */ static void ATPostAlterTypeCleanup(List **wqueue, AlteredTableInfo *tab, LOCKMODE lockmode) diff --git a/src/include/catalog/dependency.h b/src/include/catalog/dependency.h index 7da6a3942e1..0675e79d0c4 100644 --- a/src/include/catalog/dependency.h +++ b/src/include/catalog/dependency.h @@ -97,6 +97,15 @@ typedef struct ObjectAddresses ObjectAddresses; #define PERFORM_DELETION_CONCURRENT_LOCK 0x0020 /* normal drop with * concurrent lock mode */ +/* + * Attribute number used to track whole-row references, to allow + * tracking of dependencies on the shape of the referenced relation. + * + * Without this, we'd have to scan all dependent objects' expressions + * every time a column is added, dropped, or modified. + */ +#define WholeRowAttrNumber INT16_MIN + /* in dependency.c */ diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 910eef936f1..29417ff3f3e 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2437,6 +2437,7 @@ typedef enum ObjectType OBJECT_TYPE, OBJECT_USER_MAPPING, OBJECT_VIEW, + OBJECT_WHOLE_ROW, } ObjectType; /* ---------------------- diff --git a/src/test/regress/expected/indexing.out b/src/test/regress/expected/indexing.out index 4a0a652e9f6..b89f3bfc094 100644 --- a/src/test/regress/expected/indexing.out +++ b/src/test/regress/expected/indexing.out @@ -1782,6 +1782,20 @@ insert into test_pg_wholerow_index values (1, 'multiplication', 1.0); create index row_image_index on test_pg_wholerow_index ((row_image(test_pg_wholerow_index))); insert into test_pg_wholerow_index values (2, 'addition', 0); +-- test shape changes invalidate an index with whole-row references +select relfilenode as rfn_orig from pg_class where oid = 'row_image_index'::regclass \gset +alter table test_pg_wholerow_index add column d int; -- add column +select relfilenode as rfn_ac from pg_class where oid = 'row_image_index'::regclass \gset +alter table test_pg_wholerow_index alter column d set data type bigint; -- alter column type +select relfilenode as rfn_sdt from pg_class where oid = 'row_image_index'::regclass \gset +alter table test_pg_wholerow_index drop column d; -- drop column +-- Check that the relfilenode changed at every step +select :rfn_orig != :rfn_ac AND :rfn_ac != :rfn_sdt AND :rfn_sdt != relfilenode from pg_class where oid = 'row_image_index'::regclass; + ?column? +---------- + t +(1 row) + drop index row_image_index; drop function row_image(test_pg_wholerow_index); drop table test_pg_wholerow_index; diff --git a/src/test/regress/expected/object_address.out b/src/test/regress/expected/object_address.out index 4c266d7fa67..c29dd9fd2ea 100644 --- a/src/test/regress/expected/object_address.out +++ b/src/test/regress/expected/object_address.out @@ -69,7 +69,9 @@ DECLARE objtype text; BEGIN FOR objtype IN VALUES ('toast table'), ('index column'), ('sequence column'), - ('toast table column'), ('view column'), ('materialized view column') + ('toast table column'), ('view column'), ('materialized view column'), + ('index whole row'), ('sequence whole row'), ('toast table whole row'), + ('view whole row'), ('materialized view whole row') LOOP BEGIN PERFORM pg_get_object_address(objtype, '{one}', '{}'); @@ -85,6 +87,11 @@ WARNING: error for sequence column: unsupported object type "sequence column" WARNING: error for toast table column: unsupported object type "toast table column" WARNING: error for view column: unsupported object type "view column" WARNING: error for materialized view column: unsupported object type "materialized view column" +WARNING: error for index whole row: unsupported object type "index whole row" +WARNING: error for sequence whole row: unsupported object type "sequence whole row" +WARNING: error for toast table whole row: unsupported object type "toast table whole row" +WARNING: error for view whole row: unsupported object type "view whole row" +WARNING: error for materialized view whole row: unsupported object type "materialized view whole row" -- miscellaneous other errors select * from pg_get_object_address('operator of access method', '{btree,integer_ops,1}', '{int4,bool}'); ERROR: operator 1 (int4, bool) of operator family integer_ops for access method btree does not exist @@ -104,6 +111,7 @@ BEGIN ('table'), ('index'), ('sequence'), ('view'), ('materialized view'), ('foreign table'), ('table column'), ('foreign table column'), + ('table whole row'), ('foreign table whole row'), ('aggregate'), ('function'), ('procedure'), ('type'), ('cast'), ('table constraint'), ('domain constraint'), ('conversion'), ('default value'), ('operator'), ('operator class'), ('operator family'), ('rule'), ('trigger'), @@ -175,6 +183,18 @@ WARNING: error for foreign table column,{addr_nsp,zwei},{}: relation "addr_nsp" WARNING: error for foreign table column,{addr_nsp,zwei},{integer}: relation "addr_nsp" does not exist WARNING: error for foreign table column,{eins,zwei,drei},{}: schema "eins" does not exist WARNING: error for foreign table column,{eins,zwei,drei},{integer}: schema "eins" does not exist +WARNING: error for table whole row,{eins},{}: relation "eins" does not exist +WARNING: error for table whole row,{eins},{integer}: relation "eins" does not exist +WARNING: error for table whole row,{addr_nsp,zwei},{}: relation "addr_nsp.zwei" does not exist +WARNING: error for table whole row,{addr_nsp,zwei},{integer}: relation "addr_nsp.zwei" does not exist +WARNING: error for table whole row,{eins,zwei,drei},{}: cross-database references are not implemented: "eins.zwei.drei" +WARNING: error for table whole row,{eins,zwei,drei},{integer}: cross-database references are not implemented: "eins.zwei.drei" +WARNING: error for foreign table whole row,{eins},{}: relation "eins" does not exist +WARNING: error for foreign table whole row,{eins},{integer}: relation "eins" does not exist +WARNING: error for foreign table whole row,{addr_nsp,zwei},{}: relation "addr_nsp.zwei" does not exist +WARNING: error for foreign table whole row,{addr_nsp,zwei},{integer}: relation "addr_nsp.zwei" does not exist +WARNING: error for foreign table whole row,{eins,zwei,drei},{}: cross-database references are not implemented: "eins.zwei.drei" +WARNING: error for foreign table whole row,{eins,zwei,drei},{integer}: cross-database references are not implemented: "eins.zwei.drei" WARNING: error for aggregate,{eins},{}: aggregate eins(*) does not exist WARNING: error for aggregate,{eins},{integer}: aggregate eins(integer) does not exist WARNING: error for aggregate,{addr_nsp,zwei},{}: aggregate addr_nsp.zwei(*) does not exist @@ -403,7 +423,9 @@ WITH objects (type, name, args) AS (VALUES ('materialized view', '{addr_nsp, genmatview}', '{}'), ('foreign table', '{addr_nsp, genftable}', '{}'), ('table column', '{addr_nsp, gentable, b}', '{}'), + ('table whole row', '{addr_nsp, gentable}', '{}'), ('foreign table column', '{addr_nsp, genftable, a}', '{}'), + ('foreign table whole row', '{addr_nsp, genftable}', '{}'), ('aggregate', '{addr_nsp, genaggr}', '{int4}'), ('function', '{pg_catalog, pg_identify_object}', '{pg_catalog.oid, pg_catalog.oid, int4}'), ('procedure', '{addr_nsp, proc}', '{int4}'), @@ -469,6 +491,7 @@ function|pg_catalog|NULL|pg_catalog.pg_identify_object(pg_catalog.oid,pg_catalog aggregate|addr_nsp|NULL|addr_nsp.genaggr(integer)|t procedure|addr_nsp|NULL|addr_nsp.proc(integer)|t sequence|addr_nsp|gentable_a_seq|addr_nsp.gentable_a_seq|t +table whole row|addr_nsp|gentable|addr_nsp.gentable.*|t table|addr_nsp|gentable|addr_nsp.gentable|t table column|addr_nsp|gentable|addr_nsp.gentable.b|t index|addr_nsp|gentable_pkey|addr_nsp.gentable_pkey|t @@ -476,6 +499,7 @@ table|addr_nsp|parttable|addr_nsp.parttable|t index|addr_nsp|parttable_pkey|addr_nsp.parttable_pkey|t view|addr_nsp|genview|addr_nsp.genview|t materialized view|addr_nsp|genmatview|addr_nsp.genmatview|t +foreign table whole row|addr_nsp|genftable|addr_nsp.genftable.*|t foreign table|addr_nsp|genftable|addr_nsp.genftable|t foreign table column|addr_nsp|genftable|addr_nsp.genftable.a|t role|NULL|regress_addr_user|regress_addr_user|t diff --git a/src/test/regress/sql/indexing.sql b/src/test/regress/sql/indexing.sql index bbcfb365281..ec9513c6fb7 100644 --- a/src/test/regress/sql/indexing.sql +++ b/src/test/regress/sql/indexing.sql @@ -1002,6 +1002,15 @@ insert into test_pg_wholerow_index values (1, 'multiplication', 1.0); create index row_image_index on test_pg_wholerow_index ((row_image(test_pg_wholerow_index))); insert into test_pg_wholerow_index values (2, 'addition', 0); +-- test shape changes invalidate an index with whole-row references +select relfilenode as rfn_orig from pg_class where oid = 'row_image_index'::regclass \gset +alter table test_pg_wholerow_index add column d int; -- add column +select relfilenode as rfn_ac from pg_class where oid = 'row_image_index'::regclass \gset +alter table test_pg_wholerow_index alter column d set data type bigint; -- alter column type +select relfilenode as rfn_sdt from pg_class where oid = 'row_image_index'::regclass \gset +alter table test_pg_wholerow_index drop column d; -- drop column +-- Check that the relfilenode changed at every step +select :rfn_orig != :rfn_ac AND :rfn_ac != :rfn_sdt AND :rfn_sdt != relfilenode from pg_class where oid = 'row_image_index'::regclass; drop index row_image_index; drop function row_image(test_pg_wholerow_index); drop table test_pg_wholerow_index; diff --git a/src/test/regress/sql/object_address.sql b/src/test/regress/sql/object_address.sql index 653a53038e3..87b975be014 100644 --- a/src/test/regress/sql/object_address.sql +++ b/src/test/regress/sql/object_address.sql @@ -69,7 +69,9 @@ DECLARE objtype text; BEGIN FOR objtype IN VALUES ('toast table'), ('index column'), ('sequence column'), - ('toast table column'), ('view column'), ('materialized view column') + ('toast table column'), ('view column'), ('materialized view column'), + ('index whole row'), ('sequence whole row'), ('toast table whole row'), + ('view whole row'), ('materialized view whole row') LOOP BEGIN PERFORM pg_get_object_address(objtype, '{one}', '{}'); @@ -96,6 +98,7 @@ BEGIN ('table'), ('index'), ('sequence'), ('view'), ('materialized view'), ('foreign table'), ('table column'), ('foreign table column'), + ('table whole row'), ('foreign table whole row'), ('aggregate'), ('function'), ('procedure'), ('type'), ('cast'), ('table constraint'), ('domain constraint'), ('conversion'), ('default value'), ('operator'), ('operator class'), ('operator family'), ('rule'), ('trigger'), @@ -168,7 +171,9 @@ WITH objects (type, name, args) AS (VALUES ('materialized view', '{addr_nsp, genmatview}', '{}'), ('foreign table', '{addr_nsp, genftable}', '{}'), ('table column', '{addr_nsp, gentable, b}', '{}'), + ('table whole row', '{addr_nsp, gentable}', '{}'), ('foreign table column', '{addr_nsp, genftable, a}', '{}'), + ('foreign table whole row', '{addr_nsp, genftable}', '{}'), ('aggregate', '{addr_nsp, genaggr}', '{int4}'), ('function', '{pg_catalog, pg_identify_object}', '{pg_catalog.oid, pg_catalog.oid, int4}'), ('procedure', '{addr_nsp, proc}', '{int4}'), -- 2.50.1 (Apple Git-155)