From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alexey Bashtanov <bashtanov@imap.cc>
cc: pgsql-bugs@postgresql.org
Subject: Re: pg_dump losing index column collations for unique and primary
 keys
In-reply-to: <13713.1575392277@sss.pgh.pa.us>
References: <b05ce36a-cefb-ca5e-b386-a400535b1c0b@imap.cc>
 <13713.1575392277@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Tue, 03 Dec 2019 11:57:57 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <6383.1575397335.0@sss.pgh.pa.us>
Date: Tue, 03 Dec 2019 13:24:38 -0500
Message-ID: <6484.1575397478@sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <6383.1575397335.1@sss.pgh.pa.us>

I wrote:
> In short, I'd say the bug here is not pg_dump's fault at all,
> but failure to insist on collation match in ADD PRIMARY KEY
> USING INDEX.

Concretely, I think we should do the attached.

I'm not quite sure whether we should back-patch this, though.
It's been wrong since we added collations, but the main impact
of a back-patch might be to break cases that were working more
or less okay for people.

A compromise idea is to back-patch only into v12, where the issue
became quite a lot more important due to nondeterministic
collations.

Thoughts?

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff;
	name="disallow-collation-mismatch-in-ADD-USING-INDEX.patch";
	charset="us-ascii"
Content-ID: <6383.1575397335.2@sss.pgh.pa.us>
Content-Description: disallow-collation-mismatch-in-ADD-USING-INDEX.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse=
_utilcmd.c
index ee47547..b761fdf 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -2147,15 +2147,17 @@ transformIndexConstraint(Constraint *constraint, C=
reateStmtContext *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 =3D GetDefaultOpClass(attform->atttypid,
 											   index_rel->rd_rel->relam);
 				if (indclass->values[i] !=3D defopclass ||
+					attform->attcollation !=3D index_rel->rd_indcollation[i] ||
 					index_rel->rd_indoption[i] !=3D 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 1cdb7a9..645ae2c 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_replac=
ed_pkey on table cwi_test requires it
 HINT:  You can drop constraint cwi_replaced_pkey on table cwi_test instea=
d.
+-- 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 sorti=
ng 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 i=
ndex.
+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 sorti=
ng 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 i=
ndex.
 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 7659808..73a55ea 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

------- =_aaaaaaaaaa0--