From fbbf68094c5ff3d513969d072126c92932e484da Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 6 Dec 2019 11:25:09 -0500 Subject: [PATCH] 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 --- src/backend/parser/parse_utilcmd.c | 12 +++++++----- src/test/regress/expected/create_index.out | 13 +++++++++++++ src/test/regress/sql/create_index.sql | 6 ++++++ 3 files changed, 26 insertions(+), 5 deletions(-) diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index ee47547624..b761fdfd7d 100644 --- 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 1cdb7a9663..645ae2cf34 100644 --- 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 76598085f7..73a55ead4b 100644 --- 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