From: Alexey Bashtanov <bashtanov@imap.cc>
To: pgsql-bugs@postgresql.org
Subject: pg_dump losing index column collations for unique and primary keys
Date: Tue, 3 Dec 2019 16:35:20 +0000
Message-ID: <b05ce36a-cefb-ca5e-b386-a400535b1c0b@imap.cc> (raw)
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
Attachments:
[text/x-patch] pg_dump_pk_collation_v001.patch (9.5K, ../b05ce36a-cefb-ca5e-b386-a400535b1c0b@imap.cc/2-pg_dump_pk_collation_v001.patch)
download | inline diff:diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.cindex 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.cindex 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.hindex 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.plindex 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, },
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: bashtanov@imap.cc
Subject: Re: pg_dump losing index column collations for unique and primary keys
In-Reply-To: <b05ce36a-cefb-ca5e-b386-a400535b1c0b@imap.cc>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox