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
This commit is contained in:
Tom Lane 2021-04-16 12:26:50 -04:00
parent f90c708a04
commit ef387bed87
3 changed files with 10 additions and 24 deletions

View File

@ -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)

View File

@ -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)

View File

@ -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));