From eabc71acbce4c008e618a057c123ca15d8eb3eb5 Mon Sep 17 00:00:00 2001 From: Jeff Davis Date: Sat, 29 Oct 2022 12:38:42 -0700 Subject: [PATCH 1/4] Count the default collation as an unpinned dependency. Upgrading the collation provider library can affect the default collation in subtle ways, so track the dependency like any other. That will at least preserve the information about which objects might be affected, enabling administrator intervention. --- src/backend/catalog/catalog.c | 12 +++++ src/backend/catalog/dependency.c | 28 +++++------- src/backend/catalog/heap.c | 8 +--- src/backend/catalog/index.c | 4 +- src/backend/catalog/pg_type.c | 6 +-- src/backend/commands/tablecmds.c | 3 +- src/test/regress/expected/create_index.out | 52 +++++++++++++--------- 7 files changed, 58 insertions(+), 55 deletions(-) diff --git a/src/backend/catalog/catalog.c b/src/backend/catalog/catalog.c index 2abd6b007a..2cf3f38d1e 100644 --- a/src/backend/catalog/catalog.c +++ b/src/backend/catalog/catalog.c @@ -29,6 +29,7 @@ #include "catalog/namespace.h" #include "catalog/pg_auth_members.h" #include "catalog/pg_authid.h" +#include "catalog/pg_collation.h" #include "catalog/pg_database.h" #include "catalog/pg_db_role_setting.h" #include "catalog/pg_largeobject.h" @@ -355,6 +356,17 @@ IsPinnedObject(Oid classId, Oid objectId) if (classId == DatabaseRelationId) return false; + /* + * The default collation is never pinned. While logically the default + * collation should not change, the collation provider may change in + * subtle ways when upgraded. Recording the dependencies on the default + * collation makes it easier to understand what objects might be affected + * when that happens. + */ + if (classId == CollationRelationId && + objectId == DEFAULT_COLLATION_OID) + return false; + /* * All other initdb-created objects are pinned. This is overkill (the * system doesn't really depend on having every last weird datatype, for diff --git a/src/backend/catalog/dependency.c b/src/backend/catalog/dependency.c index 7f3e64b5ae..31ed7cd8a5 100644 --- a/src/backend/catalog/dependency.c +++ b/src/backend/catalog/dependency.c @@ -1805,11 +1805,9 @@ find_expr_references_walker(Node *node, /* * We must also depend on the constant's collation: it could be * different from the datatype's, if a CollateExpr was const-folded to - * a simple constant. However we can save work in the most common - * case where the collation is "default", since we know that's pinned. + * a simple constant. */ - if (OidIsValid(con->constcollid) && - con->constcollid != DEFAULT_COLLATION_OID) + if (OidIsValid(con->constcollid)) add_object_address(OCLASS_COLLATION, con->constcollid, 0, context->addrs); @@ -1905,8 +1903,7 @@ find_expr_references_walker(Node *node, add_object_address(OCLASS_TYPE, param->paramtype, 0, context->addrs); /* and its collation, just as for Consts */ - if (OidIsValid(param->paramcollid) && - param->paramcollid != DEFAULT_COLLATION_OID) + if (OidIsValid(param->paramcollid)) add_object_address(OCLASS_COLLATION, param->paramcollid, 0, context->addrs); } @@ -2009,8 +2006,7 @@ find_expr_references_walker(Node *node, add_object_address(OCLASS_TYPE, fselect->resulttype, 0, context->addrs); /* the collation might not be referenced anywhere else, either */ - if (OidIsValid(fselect->resultcollid) && - fselect->resultcollid != DEFAULT_COLLATION_OID) + if (OidIsValid(fselect->resultcollid)) add_object_address(OCLASS_COLLATION, fselect->resultcollid, 0, context->addrs); } @@ -2040,8 +2036,7 @@ find_expr_references_walker(Node *node, add_object_address(OCLASS_TYPE, relab->resulttype, 0, context->addrs); /* the collation might not be referenced anywhere else, either */ - if (OidIsValid(relab->resultcollid) && - relab->resultcollid != DEFAULT_COLLATION_OID) + if (OidIsValid(relab->resultcollid)) add_object_address(OCLASS_COLLATION, relab->resultcollid, 0, context->addrs); } @@ -2053,8 +2048,7 @@ find_expr_references_walker(Node *node, add_object_address(OCLASS_TYPE, iocoerce->resulttype, 0, context->addrs); /* the collation might not be referenced anywhere else, either */ - if (OidIsValid(iocoerce->resultcollid) && - iocoerce->resultcollid != DEFAULT_COLLATION_OID) + if (OidIsValid(iocoerce->resultcollid)) add_object_address(OCLASS_COLLATION, iocoerce->resultcollid, 0, context->addrs); } @@ -2066,8 +2060,7 @@ find_expr_references_walker(Node *node, add_object_address(OCLASS_TYPE, acoerce->resulttype, 0, context->addrs); /* the collation might not be referenced anywhere else, either */ - if (OidIsValid(acoerce->resultcollid) && - acoerce->resultcollid != DEFAULT_COLLATION_OID) + if (OidIsValid(acoerce->resultcollid)) add_object_address(OCLASS_COLLATION, acoerce->resultcollid, 0, context->addrs); /* fall through to examine arguments */ @@ -2155,8 +2148,7 @@ find_expr_references_walker(Node *node, if (OidIsValid(wc->endInRangeFunc)) add_object_address(OCLASS_PROC, wc->endInRangeFunc, 0, context->addrs); - if (OidIsValid(wc->inRangeColl) && - wc->inRangeColl != DEFAULT_COLLATION_OID) + if (OidIsValid(wc->inRangeColl)) add_object_address(OCLASS_COLLATION, wc->inRangeColl, 0, context->addrs); /* fall through to examine substructure */ @@ -2316,7 +2308,7 @@ find_expr_references_walker(Node *node, { Oid collid = lfirst_oid(ct); - if (OidIsValid(collid) && collid != DEFAULT_COLLATION_OID) + if (OidIsValid(collid)) add_object_address(OCLASS_COLLATION, collid, 0, context->addrs); } @@ -2338,7 +2330,7 @@ find_expr_references_walker(Node *node, { Oid collid = lfirst_oid(ct); - if (OidIsValid(collid) && collid != DEFAULT_COLLATION_OID) + if (OidIsValid(collid)) add_object_address(OCLASS_COLLATION, collid, 0, context->addrs); } diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c index 5b49cc5a09..b0cae59a41 100644 --- a/src/backend/catalog/heap.c +++ b/src/backend/catalog/heap.c @@ -832,9 +832,7 @@ AddNewAttributeTuples(Oid new_rel_oid, tupdesc->attrs[i].atttypid); recordDependencyOn(&myself, &referenced, DEPENDENCY_NORMAL); - /* The default collation is pinned, so don't bother recording it */ - if (OidIsValid(tupdesc->attrs[i].attcollation) && - tupdesc->attrs[i].attcollation != DEFAULT_COLLATION_OID) + if (OidIsValid(tupdesc->attrs[i].attcollation)) { ObjectAddressSet(referenced, CollationRelationId, tupdesc->attrs[i].attcollation); @@ -3367,9 +3365,7 @@ StorePartitionKey(Relation rel, ObjectAddressSet(referenced, OperatorClassRelationId, partopclass[i]); add_exact_object_address(&referenced, addrs); - /* The default collation is pinned, so don't bother recording it */ - if (OidIsValid(partcollation[i]) && - partcollation[i] != DEFAULT_COLLATION_OID) + if (OidIsValid(partcollation[i])) { ObjectAddressSet(referenced, CollationRelationId, partcollation[i]); add_exact_object_address(&referenced, addrs); diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c index 61f1d3926a..f835647379 100644 --- a/src/backend/catalog/index.c +++ b/src/backend/catalog/index.c @@ -1142,11 +1142,9 @@ index_create(Relation heapRelation, /* Store dependency on collations */ - /* The default collation is pinned, so don't bother recording it */ for (i = 0; i < indexInfo->ii_NumIndexKeyAttrs; i++) { - if (OidIsValid(collationObjectId[i]) && - collationObjectId[i] != DEFAULT_COLLATION_OID) + if (OidIsValid(collationObjectId[i])) { ObjectAddressSet(referenced, CollationRelationId, collationObjectId[i]); diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c index 896d0146ca..51a7b0fa7a 100644 --- a/src/backend/catalog/pg_type.c +++ b/src/backend/catalog/pg_type.c @@ -680,11 +680,9 @@ GenerateTypeDependencies(HeapTuple typeTuple, } /* - * Normal dependency from a domain to its collation. We know the default - * collation is pinned, so don't bother recording it. + * Normal dependency from a domain to its collation. */ - if (OidIsValid(typeForm->typcollation) && - typeForm->typcollation != DEFAULT_COLLATION_OID) + if (OidIsValid(typeForm->typcollation)) { ObjectAddressSet(referenced, CollationRelationId, typeForm->typcollation); add_exact_object_address(&referenced, addrs_normal); diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 20135ef1b0..fa3aeb5122 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -7175,8 +7175,7 @@ add_column_collation_dependency(Oid relid, int32 attnum, Oid collid) ObjectAddress myself, referenced; - /* We know the default collation is pinned, so don't bother recording it */ - if (OidIsValid(collid) && collid != DEFAULT_COLLATION_OID) + if (OidIsValid(collid)) { myself.classId = RelationRelationId; myself.objectId = relid; diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out index 6cd57e3eaa..cc45e489f0 100644 --- a/src/test/regress/expected/create_index.out +++ b/src/test/regress/expected/create_index.out @@ -2095,17 +2095,21 @@ WHERE classid = 'pg_class'::regclass AND 'concur_reindex_ind4'::regclass, 'concur_reindex_matview'::regclass) ORDER BY 1, 2; - obj | objref | deptype -------------------------------------------+------------------------------------------------------------+--------- - index concur_reindex_ind1 | constraint concur_reindex_ind1 on table concur_reindex_tab | i - index concur_reindex_ind2 | column c2 of table concur_reindex_tab | a - index concur_reindex_ind3 | column c1 of table concur_reindex_tab | a - index concur_reindex_ind3 | table concur_reindex_tab | a - index concur_reindex_ind4 | column c1 of table concur_reindex_tab | a - index concur_reindex_ind4 | column c2 of table concur_reindex_tab | a - materialized view concur_reindex_matview | schema public | n - table concur_reindex_tab | schema public | n -(8 rows) + obj | objref | deptype +-------------------------------------------------------+------------------------------------------------------------+--------- + column c2 of materialized view concur_reindex_matview | collation "default" | n + column c2 of table concur_reindex_tab | collation "default" | n + index concur_reindex_ind1 | constraint concur_reindex_ind1 on table concur_reindex_tab | i + index concur_reindex_ind2 | collation "default" | n + index concur_reindex_ind2 | column c2 of table concur_reindex_tab | a + index concur_reindex_ind3 | column c1 of table concur_reindex_tab | a + index concur_reindex_ind3 | table concur_reindex_tab | a + index concur_reindex_ind4 | collation "default" | n + index concur_reindex_ind4 | column c1 of table concur_reindex_tab | a + index concur_reindex_ind4 | column c2 of table concur_reindex_tab | a + materialized view concur_reindex_matview | schema public | n + table concur_reindex_tab | schema public | n +(12 rows) REINDEX INDEX CONCURRENTLY concur_reindex_ind1; REINDEX TABLE CONCURRENTLY concur_reindex_tab; @@ -2122,17 +2126,21 @@ WHERE classid = 'pg_class'::regclass AND 'concur_reindex_ind4'::regclass, 'concur_reindex_matview'::regclass) ORDER BY 1, 2; - obj | objref | deptype -------------------------------------------+------------------------------------------------------------+--------- - index concur_reindex_ind1 | constraint concur_reindex_ind1 on table concur_reindex_tab | i - index concur_reindex_ind2 | column c2 of table concur_reindex_tab | a - index concur_reindex_ind3 | column c1 of table concur_reindex_tab | a - index concur_reindex_ind3 | table concur_reindex_tab | a - index concur_reindex_ind4 | column c1 of table concur_reindex_tab | a - index concur_reindex_ind4 | column c2 of table concur_reindex_tab | a - materialized view concur_reindex_matview | schema public | n - table concur_reindex_tab | schema public | n -(8 rows) + obj | objref | deptype +-------------------------------------------------------+------------------------------------------------------------+--------- + column c2 of materialized view concur_reindex_matview | collation "default" | n + column c2 of table concur_reindex_tab | collation "default" | n + index concur_reindex_ind1 | constraint concur_reindex_ind1 on table concur_reindex_tab | i + index concur_reindex_ind2 | collation "default" | n + index concur_reindex_ind2 | column c2 of table concur_reindex_tab | a + index concur_reindex_ind3 | column c1 of table concur_reindex_tab | a + index concur_reindex_ind3 | table concur_reindex_tab | a + index concur_reindex_ind4 | collation "default" | n + index concur_reindex_ind4 | column c1 of table concur_reindex_tab | a + index concur_reindex_ind4 | column c2 of table concur_reindex_tab | a + materialized view concur_reindex_matview | schema public | n + table concur_reindex_tab | schema public | n +(12 rows) -- Check that comments are preserved CREATE TABLE testcomment (i int); -- 2.34.1