Fix bogus collation-version-recording logic.

recordMultipleDependencies had the wrong scope for its "version"
variable, allowing a version label to leak from the collation entry it
was meant for to subsequent non-collation entries.  This is relatively
hard to trigger because of the OID-descending order that the inputs
will normally arrive in: subsequent non-collation items will tend to
be pinned.  But it can be exhibited easily with a custom collation.

Also, don't special-case the default collation, but instead ignore
pinned-ness of a collation when we've found a version for it.  This
avoids creating useless pg_depend entries, and removes a not-very-
future-proof assumption that C, POSIX, and DEFAULT are the only
pinned collations.

A small problem is that, because the default collation may or may
not have a version, the regression tests can't assume anything about
whether dependency entries will be made for it.  This seems OK though
since it's now handled just the same as other collations, and we have
test cases for both versioned and unversioned collations.

Fixes oversights in commit 257836a75.  Thanks to Julien Rouhaud
for review.

Discussion: https://postgr.es/m/3564817.1618420687@sss.pgh.pa.us

diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 362db7fe913b7f40e2244a19dd4e27af486820f5..1217c01b8ae9864b74e221352fb90578248ab8e6 100644 (file)
--- a/src/backend/catalog/pg_depend.c
+++ b/src/backend/catalog/pg_depend.c
@@ -73,7 +73,6 @@ recordMultipleDependencies(const ObjectAddress *depender,
                max_slots,
                slot_init_count,
                slot_stored_count;
-   char       *version = NULL;
 
    if (nreferenced <= 0)
        return;                 /* nothing to do */
@@ -104,31 +103,22 @@ recordMultipleDependencies(const ObjectAddress *depender,
    slot_init_count = 0;
    for (i = 0; i < nreferenced; i++, referenced++)
    {
-       bool        ignore_systempin = false;
+       char       *version = NULL;
 
        if (record_version)
        {
            /* For now we only know how to deal with collations. */
            if (referenced->classId == CollationRelationId)
            {
-               /* C and POSIX don't need version tracking. */
+               /* These are unversioned, so don't waste cycles on them. */
                if (referenced->objectId == C_COLLATION_OID ||
                    referenced->objectId == POSIX_COLLATION_OID)
                    continue;
 
                version = get_collation_version_for_oid(referenced->objectId,
                                                        false);
-
-               /*
-                * Default collation is pinned, so we need to force recording
-                * the dependency to store the version.
-                */
-               if (referenced->objectId == DEFAULT_COLLATION_OID)
-                   ignore_systempin = true;
            }
        }
-       else
-           Assert(!version);
 
        /*
         * If the referenced object is pinned by the system, there's no real
@@ -136,7 +126,7 @@ recordMultipleDependencies(const ObjectAddress *depender,
         * version.  This saves lots of space in pg_depend, so it's worth the
         * time taken to check.
         */
-       if (!ignore_systempin && isObjectPinned(referenced, dependDesc))
+       if (version == NULL && isObjectPinned(referenced, dependDesc))
            continue;
 
        if (slot_init_count < max_slots)

diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 830fdddf244cf3fd8e1ab0ef21295210b5edad63..7f8f91b92c6bc6f7c0aacffb24377465f496259d 100644 (file)
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2026,10 +2026,10 @@ REINDEX TABLE concur_reindex_tab; -- notice
 NOTICE:  table "concur_reindex_tab" has no indexes to reindex
 REINDEX (CONCURRENTLY) TABLE concur_reindex_tab; -- notice
 NOTICE:  table "concur_reindex_tab" has no indexes that can be reindexed concurrently
-ALTER TABLE concur_reindex_tab ADD COLUMN c2 text; -- add toast index
+ALTER TABLE concur_reindex_tab ADD COLUMN c2 text COLLATE "C"; -- add toast index
 -- Normal index with integer column
 CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab(c1);
--- Normal index with text column
+-- Normal index with text column (with unversioned collation)
 CREATE INDEX concur_reindex_ind2 ON concur_reindex_tab(c2);
 -- UNIQUE index with expression
 CREATE UNIQUE INDEX concur_reindex_ind3 ON concur_reindex_tab(abs(c1));
@@ -2069,16 +2069,14 @@ WHERE classid = 'pg_class'::regclass AND
                    obj                    |                           objref                           | deptype 
 ------------------------------------------+------------------------------------------------------------+---------
  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
-(10 rows)
+(8 rows)
 
 REINDEX INDEX CONCURRENTLY concur_reindex_ind1;
 REINDEX TABLE CONCURRENTLY concur_reindex_tab;
@@ -2098,16 +2096,14 @@ WHERE classid = 'pg_class'::regclass AND
                    obj                    |                           objref                           | deptype 
 ------------------------------------------+------------------------------------------------------------+---------
  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
-(10 rows)
+(8 rows)
 
 -- Check that comments are preserved
 CREATE TABLE testcomment (i int);
@@ -2487,7 +2483,7 @@ WARNING:  cannot reindex system catalogs concurrently, skipping all
  Column |  Type   | Collation | Nullable | Default 
 --------+---------+-----------+----------+---------
  c1     | integer |           | not null | 
- c2     | text    |           |          | 
+ c2     | text    | C         |          | 
 Indexes:
     "concur_reindex_ind1" PRIMARY KEY, btree (c1)
     "concur_reindex_ind2" btree (c2)

diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 8bc76f7c6f1872fa8e8a80a1938a64f8d6c001b9..51c9a121514ac53b58a84b955ea9e2d50424d862 100644 (file)
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -797,10 +797,10 @@ CREATE TABLE concur_reindex_tab (c1 int);
 -- REINDEX
 REINDEX TABLE concur_reindex_tab; -- notice
 REINDEX (CONCURRENTLY) TABLE concur_reindex_tab; -- notice
-ALTER TABLE concur_reindex_tab ADD COLUMN c2 text; -- add toast index
+ALTER TABLE concur_reindex_tab ADD COLUMN c2 text COLLATE "C"; -- add toast index
 -- Normal index with integer column
 CREATE UNIQUE INDEX concur_reindex_ind1 ON concur_reindex_tab(c1);
--- Normal index with text column
+-- Normal index with text column (with unversioned collation)
 CREATE INDEX concur_reindex_ind2 ON concur_reindex_tab(c2);
 -- UNIQUE index with expression
 CREATE UNIQUE INDEX concur_reindex_ind3 ON concur_reindex_tab(abs(c1));