Dependencies for partitioned indexes are still a mess
I've been experimenting with trying to dump-and-restore the
regression database, which is a test case that for some reason
we don't cover in the buildfarm (pg_upgrade is not the same thing).
It seems like the dependency choices we've made for partitioned
indexes are a complete failure for this purpose.
Setup:
1. make installcheck
2. Work around the bug complained of at [1]/messages/by-id/3169466.1594841366@sss.pgh.pa.us:
psql regression -c 'drop table gtest30_1, gtest1_1'
3. pg_dump -Fc regression >regression.dump
Issue #1: "--clean" does not work
1. createdb r2
2. pg_restore -d r2 regression.dump
3. pg_restore --clean -d r2 regression.dump
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 6606; 1259 35458 INDEX idxpart32_a_idx postgres
pg_restore: error: could not execute query: ERROR: cannot drop index public.idxpart32_a_idx because index public.idxpart3_a_idx requires it
HINT: You can drop index public.idxpart3_a_idx instead.
Command was: DROP INDEX public.idxpart32_a_idx;
pg_restore: from TOC entry 6605; 1259 35454 INDEX idxpart31_a_idx postgres
pg_restore: error: could not execute query: ERROR: cannot drop index public.idxpart31_a_idx because index public.idxpart3_a_idx requires it
HINT: You can drop index public.idxpart3_a_idx instead.
Command was: DROP INDEX public.idxpart31_a_idx;
...
pg_restore: from TOC entry 6622; 2606 35509 CONSTRAINT pk52 pk52_pkey postgres
pg_restore: error: could not execute query: ERROR: cannot drop inherited constraint "pk52_pkey" of relation "pk52"
Command was: ALTER TABLE ONLY regress_indexing.pk52 DROP CONSTRAINT pk52_pkey;
pg_restore: from TOC entry 6620; 2606 35504 CONSTRAINT pk51 pk51_pkey postgres
pg_restore: error: could not execute query: ERROR: cannot drop inherited constraint "pk51_pkey" of relation "pk51"
Command was: ALTER TABLE ONLY regress_indexing.pk51 DROP CONSTRAINT pk51_pkey;
pg_restore: from TOC entry 6618; 2606 35502 CONSTRAINT pk5 pk5_pkey postgres
pg_restore: error: could not execute query: ERROR: cannot drop inherited constraint "pk5_pkey" of relation "pk5"
Command was: ALTER TABLE ONLY regress_indexing.pk5 DROP CONSTRAINT pk5_pkey;
...
(There seem to be some other problems as well, but most of the 54 complaints
are related to partitioned indexes/constraints.)
Issue #2: parallel restore does not work
1. dropdb r2; createdb r2
2. pg_restore -j8 -d r2 regression.dump
This is fairly timing-dependent, but some attempts fail with messages
like
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 6684; 2606 29166 FK CONSTRAINT fk fk_a_fkey postgres
pg_restore: error: could not execute query: ERROR: there is no unique constraint matching given keys for referenced table "pk"
Command was: ALTER TABLE fkpart3.fk
ADD CONSTRAINT fk_a_fkey FOREIGN KEY (a) REFERENCES fkpart3.pk(a);
The problem here seems to be that some commands like this:
ALTER INDEX fkpart3.pk5_pkey ATTACH PARTITION fkpart3.pk52_pkey;
are not executed soon enough, indicating that we lack dependencies
that would guarantee the restore order.
I have not analyzed these issues in any detail -- they're just bugs
I tripped over while testing parallel pg_restore. In particular
I do not know if #1 and #2 have the same root cause.
regards, tom lane
On 2020-Jul-15, Tom Lane wrote:
Issue #1: "--clean" does not work
1. createdb r2
2. pg_restore -d r2 regression.dump
3. pg_restore --clean -d r2 regression.dumppg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 6606; 1259 35458 INDEX idxpart32_a_idx postgres
pg_restore: error: could not execute query: ERROR: cannot drop index public.idxpart32_a_idx because index public.idxpart3_a_idx requires it
HINT: You can drop index public.idxpart3_a_idx instead.
Command was: DROP INDEX public.idxpart32_a_idx;
I think this problem is just that we're trying to drop a partition index
that's not droppable. This seems fixed with just leaving the dropStmt
empty, as in the attached.
One issue is that if you previously restored only that particular
partition and its indexes, but not the ATTACH command that would make it
dependent on the parent index, there would not be a DROP command to get
rid of it. Do we need to be concerned about that case? I'm inclined to
think not.
(There seem to be some other problems as well, but most of the 54 complaints
are related to partitioned indexes/constraints.)
In my run of it there's a good dozen remaining problems, all alike: we
do DROP TYPE widget CASCADE (which works) followed by DROP FUNCTION
public.widget_out(widget), which fails complaining that type widget
doesn't exist. But in reality the error is innocuous, since that
function was dropped by the DROP TYPE CASCADE anyway. You could say
that the same thing is happening with these noisy DROP INDEX of index
partitions: the complaints are right in that each partition's DROP INDEX
command doesn't actually work, but the indexes are dropped later anyway,
so the effect is the same.
Issue #2: parallel restore does not work
Looking.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
0001-Don-t-dump-DROP-stmts-for-index-partitions.patchtext/x-diff; charset=us-asciiDownload
From 8334445705c53bb0abff407ebb92ac67975a5898 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Wed, 12 Aug 2020 17:36:37 -0400
Subject: [PATCH] Don't dump DROP stmts for index partitions
They would fail to run in --clean mode anyway
---
src/bin/pg_dump/pg_dump.c | 14 +++++++++-----
src/bin/pg_dump/pg_dump.h | 2 +-
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9c8436dde6..42391b0b2c 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -16416,7 +16416,8 @@ dumpIndex(Archive *fout, IndxInfo *indxinfo)
qindxname);
}
- appendPQExpBuffer(delq, "DROP INDEX %s;\n", qqindxname);
+ if (indxinfo->parentidx == InvalidOid)
+ appendPQExpBuffer(delq, "DROP INDEX %s;\n", qqindxname);
if (indxinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
ArchiveEntry(fout, indxinfo->dobj.catId, indxinfo->dobj.dumpId,
@@ -16695,10 +16696,13 @@ dumpConstraint(Archive *fout, ConstraintInfo *coninfo)
"pg_catalog.pg_class", "INDEX",
fmtQualifiedDumpable(indxinfo));
- appendPQExpBuffer(delq, "ALTER %sTABLE ONLY %s ", foreign,
- fmtQualifiedDumpable(tbinfo));
- appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n",
- fmtId(coninfo->dobj.name));
+ if (indxinfo->parentidx == InvalidOid)
+ {
+ appendPQExpBuffer(delq, "ALTER %sTABLE ONLY %s ", foreign,
+ fmtQualifiedDumpable(tbinfo));
+ appendPQExpBuffer(delq, "DROP CONSTRAINT %s;\n",
+ fmtId(coninfo->dobj.name));
+ }
tag = psprintf("%s %s", tbinfo->dobj.name, coninfo->dobj.name);
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index da97b731b1..2f051b83d9 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -368,7 +368,7 @@ typedef struct _indxInfo
* contains both key and nonkey attributes */
bool indisclustered;
bool indisreplident;
- Oid parentidx; /* if partitioned, parent index OID */
+ Oid parentidx; /* if a partition, parent index OID */
SimplePtrList partattaches; /* if partitioned, partition attach objects */
/* if there is an associated constraint object, its dumpId: */
--
2.20.1
Hi,
On 2020-07-15 15:52:03 -0400, Tom Lane wrote:
I've been experimenting with trying to dump-and-restore the
regression database, which is a test case that for some reason
we don't cover in the buildfarm (pg_upgrade is not the same thing).
Yea, we really should have that. IIRC I was trying to add that, and
tests that compare dumps from primary / standby, and failed due to some
differences that were hard to fix.
A quick test with pg_dumpall shows some odd differences after:
1) create new cluster
2) installcheck-parallel
3) drop table gtest30_1, gtest1_1;
4) pg_dumpall > first.sql
5) recreate cluster
6) psql < first.sql > first.sql.log
7) pg_dumpall > second.sql
I've attached the diff between first.sql and second.sql. Here's the
highlights:
@@ -15392,9 +15392,9 @@
-- CREATE TABLE public.test_type_diff2_c1 (
+ int_two smallint,
int_four bigint,
- int_eight bigint,
- int_two smallint
+ int_eight bigint
)
INHERITS (public.test_type_diff2);
...
@@ -39194,10 +39194,10 @@
-- Data for Name: b_star; Type: TABLE DATA; Schema: public; Owner: andres
---COPY public.b_star (class, aa, bb, a) FROM stdin;
-b 3 mumble \N
+COPY public.b_star (class, aa, a, bb) FROM stdin;
+b 3 \N mumble
b 4 \N \N
-b \N bumble \N
+b \N \N bumble
b \N \N \N
\.@@ -323780,7 +323780,7 @@
-- Data for Name: renamecolumnanother; Type: TABLE DATA; Schema: public; Owner: andres
---COPY public.renamecolumnanother (d, a, c, w) FROM stdin;
+COPY public.renamecolumnanother (d, w, a, c) FROM stdin;
\.The primary / standby differences are caused by sequence logging. I
wonder if there's some good way to hide those, or to force them to be
the same between primary / standby, without hiding bugs.
Greetings,
Andres Freund
Attachments:
test_dump_restore.difftext/x-diff; charset=us-asciiDownload
--- /tmp/first.sql 2020-08-12 15:01:11.810862861 -0700
+++ /tmp/second.sql 2020-08-12 15:02:05.877709572 -0700
@@ -15392,9 +15392,9 @@
--
CREATE TABLE public.test_type_diff2_c1 (
+ int_two smallint,
int_four bigint,
- int_eight bigint,
- int_two smallint
+ int_eight bigint
)
INHERITS (public.test_type_diff2);
@@ -15406,9 +15406,9 @@
--
CREATE TABLE public.test_type_diff2_c2 (
- int_eight bigint,
int_two smallint,
- int_four bigint
+ int_four bigint,
+ int_eight bigint
)
INHERITS (public.test_type_diff2);
@@ -39194,10 +39194,10 @@
-- Data for Name: b_star; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.b_star (class, aa, bb, a) FROM stdin;
-b 3 mumble \N
+COPY public.b_star (class, aa, a, bb) FROM stdin;
+b 3 \N mumble
b 4 \N \N
-b \N bumble \N
+b \N \N bumble
b \N \N \N
\.
@@ -91102,10 +91102,10 @@
-- Data for Name: c_star; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.c_star (class, aa, cc, a) FROM stdin;
-c 5 hi mom \N
+COPY public.c_star (class, aa, a, cc) FROM stdin;
+c 5 \N hi mom
c 6 \N \N
-c \N hi paul \N
+c \N \N hi paul
c \N \N \N
\.
@@ -91381,22 +91381,22 @@
-- Data for Name: d_star; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.d_star (class, aa, bb, cc, dd, a) FROM stdin;
-d 7 grumble hi sunita 0 \N
-d 8 stumble hi koko \N \N
-d 9 rumble \N 1.1 \N
-d 10 \N hi kristin 10.01 \N
-d \N crumble hi boris 100.001 \N
-d 11 fumble \N \N \N
-d 12 \N hi avi \N \N
-d 13 \N \N 1000.0001 \N
-d \N tumble hi andrew \N \N
-d \N humble \N 10000.00001 \N
-d \N \N hi ginger 100000.000001 \N
+COPY public.d_star (class, aa, a, bb, cc, dd) FROM stdin;
+d 7 \N grumble hi sunita 0
+d 8 \N stumble hi koko \N
+d 9 \N rumble \N 1.1
+d 10 \N \N hi kristin 10.01
+d \N \N crumble hi boris 100.001
+d 11 \N fumble \N \N
+d 12 \N \N hi avi \N
+d 13 \N \N \N 1000.0001
+d \N \N tumble hi andrew \N
+d \N \N humble \N 10000.00001
+d \N \N \N hi ginger 100000.000001
d 14 \N \N \N \N
-d \N jumble \N \N \N
-d \N \N hi jolly \N \N
-d \N \N \N 1000000.0000001 \N
+d \N \N jumble \N \N
+d \N \N \N hi jolly \N
+d \N \N \N \N 1000000.0000001
d \N \N \N \N \N
\.
@@ -103095,14 +103095,14 @@
-- Data for Name: e_star; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.e_star (class, aa, cc, ee, e, a) FROM stdin;
-e 15 hi carol -1 \N \N
-e 16 hi bob \N \N \N
-e 17 \N -2 \N \N
-e \N hi michelle -3 \N \N
+COPY public.e_star (class, aa, a, cc, ee, e) FROM stdin;
+e 15 \N hi carol -1 \N
+e 16 \N hi bob \N \N
+e 17 \N \N -2 \N
+e \N \N hi michelle -3 \N
e 18 \N \N \N \N
-e \N hi elisa \N \N \N
-e \N \N -4 \N \N
+e \N \N hi elisa \N \N
+e \N \N \N -4 \N
\.
@@ -103174,23 +103174,23 @@
-- Data for Name: f_star; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.f_star (class, aa, cc, ee, ff, f, e, a) FROM stdin;
-f 19 hi claire -5 ((1,3),(2,4)) 10 \N \N
-f 20 hi mike -6 \N 10 \N \N
-f 21 hi marcel \N ((11,44),(22,55),(33,66)) 10 \N \N
-f 22 \N -7 ((111,555),(222,666),(333,777),(444,888)) 10 \N \N
-f \N hi keith -8 ((1111,3333),(2222,4444)) 10 \N \N
-f 24 hi marc \N \N 10 \N \N
-f 25 \N -9 \N 10 \N \N
-f 26 \N \N ((11111,33333),(22222,44444)) 10 \N \N
-f \N hi allison -10 \N 10 \N \N
-f \N hi jeff \N ((111111,333333),(222222,444444)) 10 \N \N
-f \N \N -11 ((1111111,3333333),(2222222,4444444)) 10 \N \N
-f 27 \N \N \N 10 \N \N
-f \N hi carl \N \N 10 \N \N
-f \N \N -12 \N 10 \N \N
-f \N \N \N ((11111111,33333333),(22222222,44444444)) 10 \N \N
-f \N \N \N \N 10 \N \N
+COPY public.f_star (class, aa, a, cc, ee, e, ff, f) FROM stdin;
+f 19 \N hi claire -5 \N ((1,3),(2,4)) 10
+f 20 \N hi mike -6 \N \N 10
+f 21 \N hi marcel \N \N ((11,44),(22,55),(33,66)) 10
+f 22 \N \N -7 \N ((111,555),(222,666),(333,777),(444,888)) 10
+f \N \N hi keith -8 \N ((1111,3333),(2222,4444)) 10
+f 24 \N hi marc \N \N \N 10
+f 25 \N \N -9 \N \N 10
+f 26 \N \N \N \N ((11111,33333),(22222,44444)) 10
+f \N \N hi allison -10 \N \N 10
+f \N \N hi jeff \N \N ((111111,333333),(222222,444444)) 10
+f \N \N \N -11 \N ((1111111,3333333),(2222222,4444444)) 10
+f 27 \N \N \N \N \N 10
+f \N \N hi carl \N \N \N 10
+f \N \N \N -12 \N \N 10
+f \N \N \N \N \N ((11111111,33333333),(22222222,44444444)) 10
+f \N \N \N \N \N \N 10
\.
@@ -323780,7 +323780,7 @@
-- Data for Name: renamecolumnanother; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.renamecolumnanother (d, a, c, w) FROM stdin;
+COPY public.renamecolumnanother (d, w, a, c) FROM stdin;
\.
@@ -323788,7 +323788,7 @@
-- Data for Name: renamecolumnchild; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.renamecolumnchild (d, a, w) FROM stdin;
+COPY public.renamecolumnchild (d, w, a) FROM stdin;
\.
@@ -386337,8 +386337,8 @@
-- Data for Name: test_type_diff2_c1; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.test_type_diff2_c1 (int_four, int_eight, int_two) FROM stdin;
-1 2 3
+COPY public.test_type_diff2_c1 (int_two, int_four, int_eight) FROM stdin;
+3 1 2
\.
@@ -386346,8 +386346,8 @@
-- Data for Name: test_type_diff2_c2; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.test_type_diff2_c2 (int_eight, int_two, int_four) FROM stdin;
-4 5 6
+COPY public.test_type_diff2_c2 (int_two, int_four, int_eight) FROM stdin;
+5 6 4
\.
@@ -386364,8 +386364,8 @@
-- Data for Name: test_type_diff_c; Type: TABLE DATA; Schema: public; Owner: andres
--
-COPY public.test_type_diff_c (f1, extra, f2) FROM stdin;
-1 2 3
+COPY public.test_type_diff_c (f1, f2, extra) FROM stdin;
+1 3 2
\.
Andres Freund <andres@anarazel.de> writes:
I've attached the diff between first.sql and second.sql. Here's the
highlights:
As I recall, the differences in b_star etc are expected, because
pg_dump reorders that table's columns to match its inheritance parent,
which they don't to start with because of ALTER TABLE operations.
I'm pretty sure we set it up that way deliberately ages ago, because
pg_dump used to have bugs when contending with such cases. Not sure
about a good way to mechanize recognizing that these diffs are
expected.
Dunno about test_type_diff2, but it might be a newer instance of
the same thing.
regards, tom lane
Hi,
On 2020-08-12 18:29:16 -0400, Tom Lane wrote:
Andres Freund <andres@anarazel.de> writes:
I've attached the diff between first.sql and second.sql. Here's the
highlights:As I recall, the differences in b_star etc are expected, because
pg_dump reorders that table's columns to match its inheritance parent,
which they don't to start with because of ALTER TABLE operations.
Ugh. Obviously applications shouldn't use INSERT or SELECT without a
target list, but that still seems somewhat nasty.
I guess we could script it so that we don't compare the "original" with
a restored database, but instead compare the restored version with one
restored from that. But that seems likely to hide bugs.
Given that pg_dump already re-orders the columns for DDL, could we make
it apply that re-ordering not just during the CREATE TABLE, but also
when dumping the table contents?
Greetings,
Andres Freund
On 2020-Jul-15, Tom Lane wrote:
Issue #2: parallel restore does not work
1. dropdb r2; createdb r2
2. pg_restore -j8 -d r2 regression.dumpThis is fairly timing-dependent, but some attempts fail with messages
likepg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 6684; 2606 29166 FK CONSTRAINT fk fk_a_fkey postgres
pg_restore: error: could not execute query: ERROR: there is no unique constraint matching given keys for referenced table "pk"
Command was: ALTER TABLE fkpart3.fk
ADD CONSTRAINT fk_a_fkey FOREIGN KEY (a) REFERENCES fkpart3.pk(a);
Hmm, we do make the FK constraint depend on the ATTACH for the direct
children; what I think we're lacking is dependencies on descendants
twice-removed (?) or higher. This mock patch seems to fix this problem
by adding dependencies recursively on all children of the index; I no
longer see this problem with it.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
add-recursive-deps.patchtext/x-diff; charset=us-asciiDownload
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 42391b0b2c..c78bbd7d00 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7412,6 +7412,24 @@ getExtendedStatistics(Archive *fout)
destroyPQExpBuffer(query);
}
+/* recursive bit of getConstraints */
+static void
+addConstrChildIdxDeps(DumpableObject *dobj, IndxInfo *refidx)
+{
+ SimplePtrListCell *cell;
+
+ for (cell = refidx->partattaches.head; cell; cell = cell->next)
+ {
+ DumpableObject *childobj = (DumpableObject *) cell->ptr;
+ IndexAttachInfo *attach = (IndexAttachInfo *) childobj;
+
+ addObjectDependency(dobj, childobj->dumpId);
+
+ if (attach->partitionIdx->partattaches.head != NULL)
+ addConstrChildIdxDeps(dobj, attach->partitionIdx);
+ }
+}
+
/*
* getConstraints
*
@@ -7517,25 +7535,20 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
reftable = findTableByOid(constrinfo[j].confrelid);
if (reftable && reftable->relkind == RELKIND_PARTITIONED_TABLE)
{
- IndxInfo *refidx;
Oid indexOid = atooid(PQgetvalue(res, j, i_conindid));
if (indexOid != InvalidOid)
{
for (int k = 0; k < reftable->numIndexes; k++)
{
- SimplePtrListCell *cell;
+ IndxInfo *refidx;
/* not our index? */
if (reftable->indexes[k].dobj.catId.oid != indexOid)
continue;
refidx = &reftable->indexes[k];
- for (cell = refidx->partattaches.head; cell;
- cell = cell->next)
- addObjectDependency(&constrinfo[j].dobj,
- ((DumpableObject *)
- cell->ptr)->dumpId);
+ addConstrChildIdxDeps(&constrinfo[j].dobj, refidx);
break;
}
}
Andres Freund <andres@anarazel.de> writes:
Given that pg_dump already re-orders the columns for DDL, could we make
it apply that re-ordering not just during the CREATE TABLE, but also
when dumping the table contents?
Hm, possibly. I think when this was last looked at, we didn't have any
way to get COPY to output the columns in non-physical order, but now we
do.
regards, tom lane
On 2020-Aug-12, Alvaro Herrera wrote:
Hmm, we do make the FK constraint depend on the ATTACH for the direct
children; what I think we're lacking is dependencies on descendants
twice-removed (?) or higher. This mock patch seems to fix this problem
by adding dependencies recursively on all children of the index; I no
longer see this problem with it.
After going over this some more, this analysis seems correct. Here's a
better version of the patch which seems final to me.
I'm not yet clear on whether the noisy DROP INDEX is an actual bug that
needs to be fixed, or instead it needs to be left alone.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Attachments:
v2-0001-pg_dump-fix-dependencies-on-FKs-to-multi-level-pa.patchtext/x-diff; charset=iso-8859-1Download
From 07d8af1885edafbd670efc52faa15824787a1dc2 Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Fri, 14 Aug 2020 13:26:20 -0400
Subject: [PATCH v2] pg_dump: fix dependencies on FKs to multi-level
partitioned tables
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Parallel-restoring a foreign key that references a partitioned table
with several levels of partitions can fail:
pg_restore: while PROCESSING TOC:
pg_restore: from TOC entry 6684; 2606 29166 FK CONSTRAINT fk fk_a_fkey postgres
pg_restore: error: could not execute query: ERROR: there is no unique constraint matching given keys for referenced table "pk"
Command was: ALTER TABLE fkpart3.fk
ADD CONSTRAINT fk_a_fkey FOREIGN KEY (a) REFERENCES fkpart3.pk(a);
This happens in parallel restore mode because some index partitions
aren't yet attached to the topmost partitioned index that the FK uses,
and so the index is still invalid. The current code marks the FK as
dependent on the first level of index-attach dump objects; the bug is
fixed by recursively marking the FK on their children.
Reported-by: Tom Lane <tgl@sss.pgh.pa.us>
Author: Ãlvaro Herrera <alvherre@alvh.no-ip.org>
Discussion: https://postgr.es/m/3170626.1594842723@sss.pgh.pa.us
---
src/bin/pg_dump/pg_dump.c | 39 ++++++++++++++++++++++++++++++++-------
1 file changed, 32 insertions(+), 7 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 9c8436dde6..2cb3f9b083 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -235,6 +235,7 @@ static DumpableObject *createBoundaryObjects(void);
static void addBoundaryDependencies(DumpableObject **dobjs, int numObjs,
DumpableObject *boundaryObjs);
+static void addConstrChildIdxDeps(DumpableObject *dobj, IndxInfo *refidx);
static void getDomainConstraints(Archive *fout, TypeInfo *tyinfo);
static void getTableData(DumpOptions *dopt, TableInfo *tblinfo, int numTables, char relkind);
static void makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo);
@@ -7517,25 +7518,20 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
reftable = findTableByOid(constrinfo[j].confrelid);
if (reftable && reftable->relkind == RELKIND_PARTITIONED_TABLE)
{
- IndxInfo *refidx;
Oid indexOid = atooid(PQgetvalue(res, j, i_conindid));
if (indexOid != InvalidOid)
{
for (int k = 0; k < reftable->numIndexes; k++)
{
- SimplePtrListCell *cell;
+ IndxInfo *refidx;
/* not our index? */
if (reftable->indexes[k].dobj.catId.oid != indexOid)
continue;
refidx = &reftable->indexes[k];
- for (cell = refidx->partattaches.head; cell;
- cell = cell->next)
- addObjectDependency(&constrinfo[j].dobj,
- ((DumpableObject *)
- cell->ptr)->dumpId);
+ addConstrChildIdxDeps(&constrinfo[j].dobj, refidx);
break;
}
}
@@ -7548,6 +7544,35 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables)
destroyPQExpBuffer(query);
}
+/*
+ * addConstrChildIdxDeps
+ *
+ * Recursive subroutine for getConstraints
+ *
+ * Given an object representing a foreign key constraint and an index on the
+ * partitioned table it references, mark the constraint object as dependent
+ * on the DO_INDEX_ATTACH object of each index partition, recursively
+ * drilling down to their partitions if any. This ensures that the FK is not
+ * restored until the index is fully marked valid.
+ */
+static void
+addConstrChildIdxDeps(DumpableObject *dobj, IndxInfo *refidx)
+{
+ SimplePtrListCell *cell;
+
+ Assert(dobj->objType == DO_FK_CONSTRAINT);
+
+ for (cell = refidx->partattaches.head; cell; cell = cell->next)
+ {
+ IndexAttachInfo *attach = (IndexAttachInfo *) cell->ptr;
+
+ addObjectDependency(dobj, attach->dobj.dumpId);
+
+ if (attach->partitionIdx->partattaches.head != NULL)
+ addConstrChildIdxDeps(dobj, attach->partitionIdx);
+ }
+}
+
/*
* getDomainConstraints
*
--
2.20.1
On 2020-Aug-14, Alvaro Herrera wrote:
On 2020-Aug-12, Alvaro Herrera wrote:
Hmm, we do make the FK constraint depend on the ATTACH for the direct
children; what I think we're lacking is dependencies on descendants
twice-removed (?) or higher. This mock patch seems to fix this problem
by adding dependencies recursively on all children of the index; I no
longer see this problem with it.After going over this some more, this analysis seems correct. Here's a
better version of the patch which seems final to me.
Pushed.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On 2020-Aug-12, Alvaro Herrera wrote:
On 2020-Jul-15, Tom Lane wrote:
(There seem to be some other problems as well, but most of the 54 complaints
are related to partitioned indexes/constraints.)In my run of it there's a good dozen remaining problems, all alike: we
do DROP TYPE widget CASCADE (which works) followed by DROP FUNCTION
public.widget_out(widget), which fails complaining that type widget
doesn't exist. But in reality the error is innocuous, since that
function was dropped by the DROP TYPE CASCADE anyway. You could say
that the same thing is happening with these noisy DROP INDEX of index
partitions: the complaints are right in that each partition's DROP INDEX
command doesn't actually work, but the indexes are dropped later anyway,
so the effect is the same.
I pushed the typo fix that was in this patch. Other than that, I think
this patch should not be pushed; ISTM it would break the logic.
(Consider that the partition with its index might exist beforehand and
be an independent table. If we wanted --clean to work properly, it
should definitely drop that index.)
Although I'm doubtful that it makes sense to do DROP INDEX when the
table is going to be dropped completely, even for regular tables.
--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services