Julien Rouhaud <[email protected]> writes:
> On Thu, Apr 15, 2021 at 10:06:24AM -0400, Tom Lane wrote:
>> 0001 fails for me :-(. I think that requires default collation to be C.
> Oh right, adding --no-locale to the regress opts I see that create_index is
> failing, and that's not the one I was expecting.
> We could change create_index test to create c2 with a C collation, in order to
> test that we don't track dependency on unversioned locales, and add an extra
> test in collate.linux.utf8 to check that we do track a dependency on the
> default collation as this test isn't run in the --no-locale case. The only
> case not tested would be default unversioned collation, but I'm not sure where
> to properly test that. Maybe a short leading test in collate.linux.utf8 that
> would be run on linux in that case (when getdatabaseencoding() != 'UTF8')? It
> would require an extra alternate file but it wouldn't cause too much
> maintenance problem as there should be only one test.
Since the proposed patch removes the dependency code's special-case
handling of the default collation, I don't feel like we need to jump
through hoops to prove that the default collation is tracked the
same as other collations. A regression test with alternative outputs
is a significant ongoing maintenance burden, and I do not see where
we're getting a commensurate improvement in test coverage. Especially
since, AFAICS, the two alternative outputs would essentially have to
accept both the "it works" and "it doesn't work" outcomes.
So I propose that we do 0001 below, which is my first patch plus your
suggestion about fixing up create_index.sql. This passes check-world
for me under both C and en_US.utf8 prevailing locales.
regards, tom lane
diff --git a/src/backend/catalog/pg_depend.c b/src/backend/catalog/pg_depend.c
index 362db7fe91..1217c01b8a 100644
--- 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 830fdddf24..7f8f91b92c 100644
--- 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 8bc76f7c6f..51c9a12151 100644
--- 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));
