pg_dump losing index column collations for unique and primary keys

Alexey Bashtanov <[email protected]>
Newsgroups gmane.comp.db.postgresql.bugs
Message-ID <[email protected]> 
Hello,

I found that pg_dump fails to note down the collations used primary and 
unique keys supporting indexes.
To reproduce:

create table f(a text);
create unique index f_pkey on f(a collate "C.UTF-8");
alter table f add primary key using index f_pkey;
\d f

pg_dump -t f

The only way to dump it correctly is to create indexes explicitly and 
then use them in constraint definitions.
Please could someone have a look at the patch attached. Thank you.

Best,
   Alex
pg_dump_pk_collation_v001.patch (text/x-patch, 9.5 KB) 
diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 37432a6f4b..75f1e358d8 100644
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -775,6 +775,7 @@ getDumpableObjects(DumpableObject ***objs, int *numObjs)
 /*
  * Add a dependency link to a DumpableObject
  *
+ * To force refId appear before dobj.
  * Note: duplicate dependencies are currently not eliminated
  */
 void
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 08658c8e86..01d5b37383 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7132,9 +7132,19 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
 				constrinfo[j].condomain = NULL;
 				constrinfo[j].contype = contype;
 				if (contype == 'x')
+				{
 					constrinfo[j].condef = pg_strdup(PQgetvalue(res, j, i_condef));
+				}
 				else
+				{
 					constrinfo[j].condef = NULL;
+					/*
+					 * for these types constraint definition refers to the
+					 * index definition, which we also dump, so we need
+					 * to maintain the order
+					 */
+					addObjectDependency(&constrinfo[j].dobj, indxinfo[j].dobj.dumpId);
+				}
 				constrinfo[j].confrelid = InvalidOid;
 				constrinfo[j].conindex = indxinfo[j].dobj.dumpId;
 				constrinfo[j].condeferrable = *(PQgetvalue(res, j, i_condeferrable)) == 't';
@@ -7142,12 +7152,12 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
 				constrinfo[j].conislocal = true;
 				constrinfo[j].separate = true;
 
-				indxinfo[j].indexconstraint = constrinfo[j].dobj.dumpId;
+				indxinfo[j].indexconstraint = &constrinfo[j];
 			}
 			else
 			{
 				/* Plain secondary index */
-				indxinfo[j].indexconstraint = 0;
+				indxinfo[j].indexconstraint = NULL;
 			}
 		}
 
@@ -16355,7 +16365,9 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
 {
 	DumpOptions *dopt = fout->dopt;
 	TableInfo  *tbinfo = indxinfo->indextable;
-	bool		is_constraint = (indxinfo->indexconstraint != 0);
+	bool		is_exclusion_constraint =
+		(indxinfo->indexconstraint != NULL &&
+		 indxinfo->indexconstraint->contype == 'x');
 	PQExpBuffer q;
 	PQExpBuffer delq;
 	char	   *qindxname;
@@ -16369,13 +16381,16 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
 	qindxname = pg_strdup(fmtId(indxinfo->dobj.name));
 
 	/*
-	 * If there's an associated constraint, don't dump the index per se, but
+	 * If there's an associated EXCLUDE constraint, don't dump the index per se, but
 	 * do dump any comment for it.  (This is safe because dependency ordering
 	 * will have ensured the constraint is emitted first.)	Note that the
 	 * emitted comment has to be shown as depending on the constraint, not the
 	 * index, in such cases.
+	 * We need to dump indexes that support PRIMARY and UNIQUE constraints, as
+	 * such an index may have special options like collations which cannot be
+	 * expressed in the ALTER TABLE ... ADD CONSTRAINT statement.
 	 */
-	if (!is_constraint)
+	if (!is_exclusion_constraint)
 	{
 		char	   *indstatcols = indxinfo->indstatcols;
 		char	   *indstatvals = indxinfo->indstatvals;
@@ -16469,8 +16484,9 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
 					tbinfo->dobj.namespace->dobj.name,
 					tbinfo->rolname,
 					indxinfo->dobj.catId, 0,
-					is_constraint ? indxinfo->indexconstraint :
-					indxinfo->dobj.dumpId);
+					is_exclusion_constraint ?
+						indxinfo->indexconstraint->dobj.dumpId :
+						indxinfo->dobj.dumpId);
 
 	destroyPQExpBuffer(q);
 	destroyPQExpBuffer(delq);
@@ -16635,47 +16651,9 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
 		}
 		else
 		{
-			appendPQExpBuffer(q, "%s (",
-							  coninfo->contype == 'p' ? "PRIMARY KEY" : "UNIQUE");
-			for (k = 0; k < indxinfo->indnkeyattrs; k++)
-			{
-				int			indkey = (int) indxinfo->indkeys[k];
-				const char *attname;
-
-				if (indkey == InvalidAttrNumber)
-					break;
-				attname = getAttrName(indkey, tbinfo);
-
-				appendPQExpBuffer(q, "%s%s",
-								  (k == 0) ? "" : ", ",
-								  fmtId(attname));
-			}
-
-			if (indxinfo->indnkeyattrs < indxinfo->indnattrs)
-				appendPQExpBufferStr(q, ") INCLUDE (");
-
-			for (k = indxinfo->indnkeyattrs; k < indxinfo->indnattrs; k++)
-			{
-				int			indkey = (int) indxinfo->indkeys[k];
-				const char *attname;
-
-				if (indkey == InvalidAttrNumber)
-					break;
-				attname = getAttrName(indkey, tbinfo);
-
-				appendPQExpBuffer(q, "%s%s",
-								  (k == indxinfo->indnkeyattrs) ? "" : ", ",
-								  fmtId(attname));
-			}
-
-			appendPQExpBufferChar(q, ')');
-
-			if (nonemptyReloptions(indxinfo->indreloptions))
-			{
-				appendPQExpBufferStr(q, " WITH (");
-				appendReloptionsArrayAH(q, indxinfo->indreloptions, "", fout);
-				appendPQExpBufferChar(q, ')');
-			}
+			appendPQExpBuffer(q, "%s USING INDEX %s",
+							  coninfo->contype == 'p' ? "PRIMARY KEY" : "UNIQUE",
+							  fmtId(indxinfo->dobj.name));
 
 			if (coninfo->condeferrable)
 			{
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index 7b2c1524a5..7984566b93 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -351,6 +351,29 @@ typedef struct _tableDataInfo
 	char	   *filtercond;		/* WHERE condition to limit rows dumped */
 } TableDataInfo;
 
+/*
+ * struct ConstraintInfo is used for all constraint types.  However we
+ * use a different objType for foreign key constraints, to make it easier
+ * to sort them the way we want.
+ *
+ * Note: condeferrable and condeferred are currently only valid for
+ * unique/primary-key constraints.  Otherwise that info is in condef.
+ */
+typedef struct _constraintInfo
+{
+	DumpableObject dobj;
+	TableInfo  *contable;		/* NULL if domain constraint */
+	TypeInfo   *condomain;		/* NULL if table constraint */
+	char		contype;
+	char	   *condef;			/* definition, if CHECK or FOREIGN KEY */
+	Oid			confrelid;		/* referenced table, if FOREIGN KEY */
+	DumpId		conindex;		/* identifies associated index if any */
+	bool		condeferrable;	/* true if constraint is DEFERRABLE */
+	bool		condeferred;	/* true if constraint is INITIALLY DEFERRED */
+	bool		conislocal;		/* true if constraint has local definition */
+	bool		separate;		/* true if must dump as separate item */
+} ConstraintInfo;
+
 typedef struct _indxInfo
 {
 	DumpableObject dobj;
@@ -369,8 +392,8 @@ typedef struct _indxInfo
 	Oid			parentidx;		/* if partitioned, parent index OID */
 	SimplePtrList partattaches;	/* if partitioned, partition attach objects */
 
-	/* if there is an associated constraint object, its dumpId: */
-	DumpId		indexconstraint;
+	/* an associated constraint object, or NULL if none */
+	ConstraintInfo *indexconstraint;
 } IndxInfo;
 
 typedef struct _indexAttachInfo
@@ -427,29 +450,6 @@ typedef struct _evttriggerInfo
 	char		evtenabled;
 } EventTriggerInfo;
 
-/*
- * struct ConstraintInfo is used for all constraint types.  However we
- * use a different objType for foreign key constraints, to make it easier
- * to sort them the way we want.
- *
- * Note: condeferrable and condeferred are currently only valid for
- * unique/primary-key constraints.  Otherwise that info is in condef.
- */
-typedef struct _constraintInfo
-{
-	DumpableObject dobj;
-	TableInfo  *contable;		/* NULL if domain constraint */
-	TypeInfo   *condomain;		/* NULL if table constraint */
-	char		contype;
-	char	   *condef;			/* definition, if CHECK or FOREIGN KEY */
-	Oid			confrelid;		/* referenced table, if FOREIGN KEY */
-	DumpId		conindex;		/* identifies associated index if any */
-	bool		condeferrable;	/* true if constraint is DEFERRABLE */
-	bool		condeferred;	/* true if constraint is INITIALLY DEFERRED */
-	bool		conislocal;		/* true if constraint has local definition */
-	bool		separate;		/* true if must dump as separate item */
-} ConstraintInfo;
-
 typedef struct _procLangInfo
 {
 	DumpableObject dobj;
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index 1163420491..9938e2754a 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -615,10 +615,26 @@ my %tests = (
 		},
 	},
 
+	'CREATE UNIQUE INDEX ON test_table ... (for primary key)' => {
+		regexp => qr/^
+			\QCREATE UNIQUE INDEX test_table_pkey ON dump_test.test_table USING btree (col1);\E
+			/xm,
+		like => {
+			%full_runs,
+			%dump_test_schema_runs,
+			only_dump_test_table => 1,
+			section_post_data    => 1,
+		},
+		unlike => {
+			exclude_dump_test_schema => 1,
+			exclude_test_table       => 1,
+		},
+	},
+
 	'ALTER TABLE ONLY test_table ADD CONSTRAINT ... PRIMARY KEY' => {
 		regexp => qr/^
 			\QALTER TABLE ONLY dump_test.test_table\E \n^\s+
-			\QADD CONSTRAINT test_table_pkey PRIMARY KEY (col1);\E
+			\QADD CONSTRAINT test_table_pkey PRIMARY KEY USING INDEX test_table_pkey;\E
 			/xm,
 		like => {
 			%full_runs,
@@ -2607,6 +2623,17 @@ my %tests = (
 		},
 	},
 
+	'CREATE UNIQUE INDEX ON measurement ... (for primary key)' => {
+		all_runs     => 1,
+		catch_all    => 'CREATE ... commands',
+		regexp => qr/^
+			\QCREATE UNIQUE INDEX measurement_pkey ON ONLY dump_test.measurement USING btree (city_id, logdate);\E
+		/xm,
+		like =>
+		  { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
+		unlike => { exclude_dump_test_schema => 1, },
+	},
+
 	'ALTER TABLE measurement PRIMARY KEY' => {
 		all_runs     => 1,
 		catch_all    => 'CREATE ... commands',
@@ -2615,7 +2642,7 @@ my %tests = (
 		  'ALTER TABLE dump_test.measurement ADD PRIMARY KEY (city_id, logdate);',
 		regexp => qr/^
 			\QALTER TABLE ONLY dump_test.measurement\E \n^\s+
-			\QADD CONSTRAINT measurement_pkey PRIMARY KEY (city_id, logdate);\E
+			\QADD CONSTRAINT measurement_pkey PRIMARY KEY USING INDEX measurement_pkey;\E
 		/xm,
 		like =>
 		  { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.