From 62215de2925705bc607635e45ff800364456b1a1 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sat, 29 Mar 2014 17:34:00 -0400 Subject: [PATCH] Fix dumping of a materialized view that depends on a table's primary key. It is possible for a view or materialized view to depend on a table's primary key, if the view query relies on functional dependency to abbreviate a GROUP BY list. This is problematic for pg_dump since we ordinarily want to dump view definitions in the pre-data section but indexes in post-data. pg_dump knows how to deal with this situation for regular views, by breaking the view's ON SELECT rule apart from the view proper. But it had not been taught what to do about materialized views, and in fact mistakenly dumped them as regular views in such cases, as seen in bug #9616 from Jesse Denardo. If we had CREATE OR REPLACE MATERIALIZED VIEW, we could fix this in a manner analogous to what's done for regular views; but we don't yet, and we'd not back-patch such a thing into 9.3 anyway. As a hopefully- temporary workaround, break the circularity by postponing the matview into post-data altogether when this case occurs. --- src/bin/pg_dump/pg_dump.c | 5 ++- src/bin/pg_dump/pg_dump.h | 5 +-- src/bin/pg_dump/pg_dump_sort.c | 64 +++++++++++++++++++++++++++++++--- 3 files changed, 67 insertions(+), 7 deletions(-) diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 4001f3f396..2653ef052b 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -4796,6 +4796,8 @@ getTables(Archive *fout, int *numTables) selectDumpableTable(&tblinfo[i]); tblinfo[i].interesting = tblinfo[i].dobj.dump; + tblinfo[i].postponed_def = false; /* might get set during sort */ + /* * Read-lock target tables to make sure they aren't DROPPED or altered * in schema before we get around to dumping them. @@ -13611,7 +13613,8 @@ dumpTableSchema(Archive *fout, TableInfo *tbinfo) (tbinfo->relkind == RELKIND_VIEW) ? NULL : tbinfo->reltablespace, tbinfo->rolname, (strcmp(reltypename, "TABLE") == 0) ? tbinfo->hasoids : false, - reltypename, SECTION_PRE_DATA, + reltypename, + tbinfo->postponed_def ? SECTION_POST_DATA : SECTION_PRE_DATA, q->data, delq->data, NULL, NULL, 0, NULL, NULL); diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index 398eca1a09..e01015eb0a 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -241,7 +241,7 @@ typedef struct _tableInfo char *reltablespace; /* relation tablespace */ char *reloptions; /* options specified by WITH (...) */ char *checkoption; /* WITH CHECK OPTION */ - char *toast_reloptions; /* ditto, for the TOAST table */ + char *toast_reloptions; /* WITH options for the TOAST table */ bool hasindex; /* does it have any indexes? */ bool hasrules; /* does it have any rules? */ bool hastriggers; /* does it have any triggers? */ @@ -254,9 +254,10 @@ typedef struct _tableInfo /* these two are set only if table is a sequence owned by a column: */ Oid owning_tab; /* OID of table owning sequence */ int owning_col; /* attr # of column owning sequence */ - int relpages; + int relpages; /* table's size in pages (from pg_class) */ bool interesting; /* true if need to collect more data */ + bool postponed_def; /* matview must be postponed into post-data */ /* * These fields are computed only if we decide the table is interesting diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c index bda35359c1..359093214a 100644 --- a/src/bin/pg_dump/pg_dump_sort.c +++ b/src/bin/pg_dump/pg_dump_sort.c @@ -798,6 +798,7 @@ repairTypeFuncLoop(DumpableObject *typeobj, DumpableObject *funcobj) * will be an implicit dependency in the other direction, we need to break * the loop. If there are no other objects in the loop then we can remove * the implicit dependency and leave the ON SELECT rule non-separate. + * This applies to matviews, as well. */ static void repairViewRuleLoop(DumpableObject *viewobj, @@ -814,7 +815,9 @@ repairViewRuleLoop(DumpableObject *viewobj, * Because findLoop() finds shorter cycles before longer ones, it's likely * that we will have previously fired repairViewRuleLoop() and removed the * rule's dependency on the view. Put it back to ensure the rule won't be - * emitted before the view... + * emitted before the view. + * + * Note: this approach does *not* work for matviews, at the moment. */ static void repairViewRuleMultiLoop(DumpableObject *viewobj, @@ -841,6 +844,30 @@ repairViewRuleMultiLoop(DumpableObject *viewobj, addObjectDependency(ruleobj, postDataBoundId); } +/* + * If a matview is involved in a multi-object loop, we can't currently fix + * that by splitting off the rule. As a stopgap, we try to fix it by + * dropping the constraint that the matview be dumped in the pre-data section. + * This is sufficient to handle cases where a matview depends on some unique + * index, as can happen if it has a GROUP BY for example. + * + * Note that the "next object" is not necessarily the matview itself; + * it could be the matview's rowtype, for example. We may come through here + * several times while removing all the pre-data linkages. + */ +static void +repairMatViewBoundaryMultiLoop(DumpableObject *matviewobj, + DumpableObject *boundaryobj, + DumpableObject *nextobj) +{ + TableInfo *matviewinfo = (TableInfo *) matviewobj; + + /* remove boundary's dependency on object after it in loop */ + removeObjectDependency(boundaryobj, nextobj->dumpId); + /* mark matview as postponed into post-data section */ + matviewinfo->postponed_def = true; +} + /* * Because we make tables depend on their CHECK constraints, while there * will be an automatic dependency in the other direction, we need to break @@ -956,10 +983,12 @@ repairDependencyLoop(DumpableObject **loop, return; } - /* View and its ON SELECT rule */ + /* View (including matview) and its ON SELECT rule */ if (nLoop == 2 && loop[0]->objType == DO_TABLE && loop[1]->objType == DO_RULE && + (((TableInfo *) loop[0])->relkind == 'v' || /* RELKIND_VIEW */ + ((TableInfo *) loop[0])->relkind == 'm') && /* RELKIND_MATVIEW */ ((RuleInfo *) loop[1])->ev_type == '1' && ((RuleInfo *) loop[1])->is_instead && ((RuleInfo *) loop[1])->ruletable == (TableInfo *) loop[0]) @@ -970,6 +999,8 @@ repairDependencyLoop(DumpableObject **loop, if (nLoop == 2 && loop[1]->objType == DO_TABLE && loop[0]->objType == DO_RULE && + (((TableInfo *) loop[1])->relkind == 'v' || /* RELKIND_VIEW */ + ((TableInfo *) loop[1])->relkind == 'm') && /* RELKIND_MATVIEW */ ((RuleInfo *) loop[0])->ev_type == '1' && ((RuleInfo *) loop[0])->is_instead && ((RuleInfo *) loop[0])->ruletable == (TableInfo *) loop[1]) @@ -978,12 +1009,13 @@ repairDependencyLoop(DumpableObject **loop, return; } - /* Indirect loop involving view and ON SELECT rule */ + /* Indirect loop involving view (but not matview) and ON SELECT rule */ if (nLoop > 2) { for (i = 0; i < nLoop; i++) { - if (loop[i]->objType == DO_TABLE) + if (loop[i]->objType == DO_TABLE && + ((TableInfo *) loop[i])->relkind == 'v') /* RELKIND_VIEW */ { for (j = 0; j < nLoop; j++) { @@ -1000,6 +1032,30 @@ repairDependencyLoop(DumpableObject **loop, } } + /* Indirect loop involving matview and data boundary */ + if (nLoop > 2) + { + for (i = 0; i < nLoop; i++) + { + if (loop[i]->objType == DO_TABLE && + ((TableInfo *) loop[i])->relkind == 'm') /* RELKIND_MATVIEW */ + { + for (j = 0; j < nLoop; j++) + { + if (loop[j]->objType == DO_PRE_DATA_BOUNDARY) + { + DumpableObject *nextobj; + + nextobj = (j < nLoop - 1) ? loop[j + 1] : loop[0]; + repairMatViewBoundaryMultiLoop(loop[i], loop[j], + nextobj); + return; + } + } + } + } + } + /* Table and CHECK constraint */ if (nLoop == 2 && loop[0]->objType == DO_TABLE &&