From b304b2b65fde057a35286adf3ea69f5e154d1878 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Thu, 17 Oct 2019 09:58:01 +0200 Subject: [PATCH] Fix parallel restore of FKs to partitioned tables When an FK constraint is created, it needs the index on the referenced table to exist and be valid. When doing parallel pg_restore and the referenced table was partitioned, this condition can sometimes not be met, because pg_dump didn't emit sufficient object dependencies to ensure so; this means that parallel pg_restore would fail in certain conditions. Fix by having pg_dump make the FK constraint object dependent on the partition attachment objects for the constraint's referenced index. This has been broken since f56f8f8da6af, so backpatch to Postgres 12. Discussion: https://postgr.es/m/20191005224333.GA9738@alvherre.pgsql --- src/bin/pg_dump/common.c | 3 +++ src/bin/pg_dump/pg_dump.c | 42 ++++++++++++++++++++++++++++-- src/bin/pg_dump/pg_dump.h | 2 ++ src/fe_utils/simple_list.c | 21 +++++++++++++++ src/include/fe_utils/simple_list.h | 19 +++++++++++--- 5 files changed, 82 insertions(+), 5 deletions(-) diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c index 02a865f456..3549f7bc08 100644 --- a/src/bin/pg_dump/common.c +++ b/src/bin/pg_dump/common.c @@ -412,6 +412,9 @@ flagInhIndexes(Archive *fout, TableInfo tblinfo[], int numTables) addObjectDependency(&attachinfo[k].dobj, parentidx->indextable->dobj.dumpId); + /* keep track of the list of partitions in the parent index */ + simple_ptr_list_append(&parentidx->partattaches, &attachinfo[k].dobj); + k++; } } diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index a9c868b9af..33e58fa287 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -7109,6 +7109,7 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables) indxinfo[j].indisclustered = (PQgetvalue(res, j, i_indisclustered)[0] == 't'); indxinfo[j].indisreplident = (PQgetvalue(res, j, i_indisreplident)[0] == 't'); indxinfo[j].parentidx = atooid(PQgetvalue(res, j, i_parentidx)); + indxinfo[j].partattaches = (SimplePtrList) { NULL, NULL }; contype = *(PQgetvalue(res, j, i_contype)); if (contype == 'p' || contype == 'u' || contype == 'x') @@ -7241,6 +7242,7 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables) i_conoid, i_conname, i_confrelid, + i_conindid, i_condef; int ntups; @@ -7266,7 +7268,7 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables) resetPQExpBuffer(query); if (fout->remoteVersion >= 110000) appendPQExpBuffer(query, - "SELECT tableoid, oid, conname, confrelid, " + "SELECT tableoid, oid, conname, confrelid, conindid, " "pg_catalog.pg_get_constraintdef(oid) AS condef " "FROM pg_catalog.pg_constraint " "WHERE conrelid = '%u'::pg_catalog.oid " @@ -7275,7 +7277,7 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables) tbinfo->dobj.catId.oid); else appendPQExpBuffer(query, - "SELECT tableoid, oid, conname, confrelid, " + "SELECT tableoid, oid, conname, confrelid, 0 as conindid, " "pg_catalog.pg_get_constraintdef(oid) AS condef " "FROM pg_catalog.pg_constraint " "WHERE conrelid = '%u'::pg_catalog.oid " @@ -7289,12 +7291,15 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables) i_conoid = PQfnumber(res, "oid"); i_conname = PQfnumber(res, "conname"); i_confrelid = PQfnumber(res, "confrelid"); + i_conindid = PQfnumber(res, "conindid"); i_condef = PQfnumber(res, "condef"); constrinfo = (ConstraintInfo *) pg_malloc(ntups * sizeof(ConstraintInfo)); for (j = 0; j < ntups; j++) { + TableInfo *reftable; + constrinfo[j].dobj.objType = DO_FK_CONSTRAINT; constrinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_contableoid)); constrinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_conoid)); @@ -7311,6 +7316,39 @@ getConstraints(Archive *fout, TableInfo tblinfo[], int numTables) constrinfo[j].condeferred = false; constrinfo[j].conislocal = true; constrinfo[j].separate = true; + + /* + * Restoring an FK that points to a partitioned table requires + * that all partition indexes have been attached beforehand. + * Ensure that happens by making the constraint depend on each + * index partition attach object. + */ + 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; + + /* 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); + break; + } + } + } } PQclear(res); diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index c3c2ea1473..ce04f914e1 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -371,6 +371,8 @@ typedef struct _indxInfo bool indisclustered; bool indisreplident; 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; } IndxInfo; diff --git a/src/fe_utils/simple_list.c b/src/fe_utils/simple_list.c index 8d605140a7..2232e8db73 100644 --- a/src/fe_utils/simple_list.c +++ b/src/fe_utils/simple_list.c @@ -114,3 +114,24 @@ simple_string_list_not_touched(SimpleStringList *list) } return NULL; } + +/* + * Append a pointer to the list. + * + * Caller must ensure that the pointer remains valid. + */ +void +simple_ptr_list_append(SimplePtrList *list, void *ptr) +{ + SimplePtrListCell *cell; + + cell = (SimplePtrListCell *) pg_malloc(sizeof(SimplePtrListCell)); + cell->next = NULL; + cell->ptr = ptr; + + if (list->tail) + list->tail->next = cell; + else + list->head = cell; + list->tail = cell; +} diff --git a/src/include/fe_utils/simple_list.h b/src/include/fe_utils/simple_list.h index 8a95cbb3a8..04dfa2cecf 100644 --- a/src/include/fe_utils/simple_list.h +++ b/src/include/fe_utils/simple_list.h @@ -2,9 +2,9 @@ * * Simple list facilities for frontend code * - * Data structures for simple lists of OIDs and strings. The support for - * these is very primitive compared to the backend's List facilities, but - * it's all we need in, eg, pg_dump. + * Data structures for simple lists of OIDs, strings, and pointers. The + * support for these is very primitive compared to the backend's List + * facilities, but it's all we need in, eg, pg_dump. * * * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group @@ -43,6 +43,17 @@ typedef struct SimpleStringList SimpleStringListCell *tail; } SimpleStringList; +typedef struct SimplePtrListCell +{ + struct SimplePtrListCell *next; + void *ptr; +} SimplePtrListCell; + +typedef struct SimplePtrList +{ + SimplePtrListCell *head; + SimplePtrListCell *tail; +} SimplePtrList; extern void simple_oid_list_append(SimpleOidList *list, Oid val); extern bool simple_oid_list_member(SimpleOidList *list, Oid val); @@ -52,4 +63,6 @@ extern bool simple_string_list_member(SimpleStringList *list, const char *val); extern const char *simple_string_list_not_touched(SimpleStringList *list); +extern void simple_ptr_list_append(SimplePtrList *list, void *val); + #endif /* SIMPLE_LIST_H */ -- 2.40.0