Disallow non-default collation in ADD PRIMARY KEY/UNIQUE USING INDEX.

When creating a uniqueness constraint using a pre-existing index,
we have always required that the index have the same properties you'd
get if you just let a new index get built.  However, when collations
were added, we forgot to add the index's collation to that check.

It's hard to trip over this without intentionally trying to break it:
you'd have to explicitly specify a different collation in CREATE
INDEX, then convert it to a pkey or unique constraint.  Still, if you
did that, pg_dump would emit a script that fails to reproduce the
index's collation.  The main practical problem is that after a
pg_upgrade the index would be corrupt, because its actual physical
order wouldn't match what pg_index says.  A more theoretical issue,
which is new as of v12, is that if you create the index with a
nondeterministic collation then it wouldn't be enforcing the normal
notion of uniqueness, causing the constraint to mean something
different from a normally-created constraint.

To fix, just add collation to the conditions checked for index
acceptability in ADD PRIMARY KEY/UNIQUE USING INDEX.  We won't try
to clean up after anybody who's already created such a situation;
it seems improbable enough to not be worth the effort involved.
(If you do get into trouble, a REINDEX should be enough to fix it.)

In principle this is a long-standing bug, but I chose not to
back-patch --- the odds of causing trouble seem about as great
as the odds of preventing it, and both risks are very low anyway.

Per report from Alexey Bashtanov, though this is not his preferred
fix.

Discussion: https://postgr.es/m/b05ce36a-cefb-ca5e-b386-a400535b1c0b@imap.cc

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index ee475476248a677f8300b5e52d8caefe2790eeb7..b761fdfd7d73856ff80cb93d4e897d8567a0a302 100644 (file)
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -2147,15 +2147,17 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
            if (i < index_form->indnkeyatts)
            {
                /*
-                * Insist on default opclass and sort options.  While the
-                * index would still work as a constraint with non-default
-                * settings, it might not provide exactly the same uniqueness
-                * semantics as you'd get from a normally-created constraint;
-                * and there's also the dump/reload problem mentioned above.
+                * Insist on default opclass, collation, and sort options.
+                * While the index would still work as a constraint with
+                * non-default settings, it might not provide exactly the same
+                * uniqueness semantics as you'd get from a normally-created
+                * constraint; and there's also the dump/reload problem
+                * mentioned above.
                 */
                defopclass = GetDefaultOpClass(attform->atttypid,
                                               index_rel->rd_rel->relam);
                if (indclass->values[i] != defopclass ||
+                   attform->attcollation != index_rel->rd_indcollation[i] ||
                    index_rel->rd_indoption[i] != 0)
                    ereport(ERROR,
                            (errcode(ERRCODE_WRONG_OBJECT_TYPE),

diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 1cdb7a96630305f06cf976601ded9853e66a7354..645ae2cf343caefa556111833bd3402319273ce6 100644 (file)
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -1479,6 +1479,19 @@ primary key, btree, for table "public.cwi_test"
 DROP INDEX cwi_replaced_pkey;  -- Should fail; a constraint depends on it
 ERROR:  cannot drop index cwi_replaced_pkey because constraint cwi_replaced_pkey on table cwi_test requires it
 HINT:  You can drop constraint cwi_replaced_pkey on table cwi_test instead.
+-- Check that non-default index options are rejected
+CREATE UNIQUE INDEX cwi_uniq3_idx ON cwi_test(a desc);
+ALTER TABLE cwi_test ADD UNIQUE USING INDEX cwi_uniq3_idx;  -- fail
+ERROR:  index "cwi_uniq3_idx" column number 1 does not have default sorting behavior
+LINE 1: ALTER TABLE cwi_test ADD UNIQUE USING INDEX cwi_uniq3_idx;
+                                 ^
+DETAIL:  Cannot create a primary key or unique constraint using such an index.
+CREATE UNIQUE INDEX cwi_uniq4_idx ON cwi_test(b collate "POSIX");
+ALTER TABLE cwi_test ADD UNIQUE USING INDEX cwi_uniq4_idx;  -- fail
+ERROR:  index "cwi_uniq4_idx" column number 1 does not have default sorting behavior
+LINE 1: ALTER TABLE cwi_test ADD UNIQUE USING INDEX cwi_uniq4_idx;
+                                 ^
+DETAIL:  Cannot create a primary key or unique constraint using such an index.
 DROP TABLE cwi_test;
 -- ADD CONSTRAINT USING INDEX is forbidden on partitioned tables
 CREATE TABLE cwi_test(a int) PARTITION BY hash (a);

diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index 76598085f7bbc4e78aa8f7d8445d2e3110c20633..73a55ead4bf6d3abf02f84b150c5f5886d7d9211 100644 (file)
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -538,6 +538,12 @@ ALTER TABLE cwi_test DROP CONSTRAINT cwi_uniq_idx,
 
 DROP INDEX cwi_replaced_pkey;  -- Should fail; a constraint depends on it
 
+-- Check that non-default index options are rejected
+CREATE UNIQUE INDEX cwi_uniq3_idx ON cwi_test(a desc);
+ALTER TABLE cwi_test ADD UNIQUE USING INDEX cwi_uniq3_idx;  -- fail
+CREATE UNIQUE INDEX cwi_uniq4_idx ON cwi_test(b collate "POSIX");
+ALTER TABLE cwi_test ADD UNIQUE USING INDEX cwi_uniq4_idx;  -- fail
+
 DROP TABLE cwi_test;
 
 -- ADD CONSTRAINT USING INDEX is forbidden on partitioned tables