From: Jeff Davis Date: Fri, 28 Mar 2025 23:12:55 +0000 (-0700) Subject: Matview statistics depend on matview data. X-Git-Tag: REL_18_BETA1~388 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=a0a4601765;p=thirdparty%2Fpostgresql.git Matview statistics depend on matview data. REFRESH MATERIALIZED VIEW replaces the storage, which resets statistics, so statistics must be restored afterward. If both statistics and data are being dumped for a materialized view, add a dependency from the former to the latter. Defer the statistics to SECTION_POST_DATA, and use RESTORE_PASS_POST_ACL. Reported-by: Ashutosh Bapat Reviewed-by: Ashutosh Bapat Discussion: https://postgr.es/m/CAExHW5s47kmubpbbRJzSM-Zfe0Tj2O3GBagB7YAyE8rQ-V24Uw@mail.gmail.com --- diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 82d51c89ac6..1d131e5a57d 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -72,7 +72,7 @@ static void processEncodingEntry(ArchiveHandle *AH, TocEntry *te); static void processStdStringsEntry(ArchiveHandle *AH, TocEntry *te); static void processSearchPathEntry(ArchiveHandle *AH, TocEntry *te); static int _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH); -static RestorePass _tocEntryRestorePass(TocEntry *te); +static RestorePass _tocEntryRestorePass(ArchiveHandle *AH, TocEntry *te); static bool _tocEntryIsACL(TocEntry *te); static void _disableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te); static void _enableTriggersIfNecessary(ArchiveHandle *AH, TocEntry *te); @@ -102,7 +102,8 @@ static void pending_list_append(TocEntry *l, TocEntry *te); static void pending_list_remove(TocEntry *te); static int TocEntrySizeCompareQsort(const void *p1, const void *p2); static int TocEntrySizeCompareBinaryheap(void *p1, void *p2, void *arg); -static void move_to_ready_heap(TocEntry *pending_list, +static void move_to_ready_heap(ArchiveHandle *AH, + TocEntry *pending_list, binaryheap *ready_heap, RestorePass pass); static TocEntry *pop_next_work_item(binaryheap *ready_heap, @@ -748,7 +749,7 @@ RestoreArchive(Archive *AHX) if ((te->reqs & (REQ_SCHEMA | REQ_DATA | REQ_STATS)) == 0) continue; /* ignore if not to be dumped at all */ - switch (_tocEntryRestorePass(te)) + switch (_tocEntryRestorePass(AH, te)) { case RESTORE_PASS_MAIN: (void) restore_toc_entry(AH, te, false); @@ -767,7 +768,7 @@ RestoreArchive(Archive *AHX) for (te = AH->toc->next; te != AH->toc; te = te->next) { if ((te->reqs & (REQ_SCHEMA | REQ_DATA | REQ_STATS)) != 0 && - _tocEntryRestorePass(te) == RESTORE_PASS_ACL) + _tocEntryRestorePass(AH, te) == RESTORE_PASS_ACL) (void) restore_toc_entry(AH, te, false); } } @@ -777,7 +778,7 @@ RestoreArchive(Archive *AHX) for (te = AH->toc->next; te != AH->toc; te = te->next) { if ((te->reqs & (REQ_SCHEMA | REQ_DATA | REQ_STATS)) != 0 && - _tocEntryRestorePass(te) == RESTORE_PASS_POST_ACL) + _tocEntryRestorePass(AH, te) == RESTORE_PASS_POST_ACL) (void) restore_toc_entry(AH, te, false); } } @@ -3219,7 +3220,7 @@ _tocEntryRequired(TocEntry *te, teSection curSection, ArchiveHandle *AH) * See notes with the RestorePass typedef in pg_backup_archiver.h. */ static RestorePass -_tocEntryRestorePass(TocEntry *te) +_tocEntryRestorePass(ArchiveHandle *AH, TocEntry *te) { /* "ACL LANGUAGE" was a crock emitted only in PG 7.4 */ if (strcmp(te->desc, "ACL") == 0 || @@ -3240,6 +3241,26 @@ _tocEntryRestorePass(TocEntry *te) strncmp(te->tag, "EVENT TRIGGER ", 14) == 0) return RESTORE_PASS_POST_ACL; + /* + * If statistics data is dependent on materialized view data, it must be + * deferred to RESTORE_PASS_POST_ACL. + */ + if (strcmp(te->desc, "STATISTICS DATA") == 0) + { + for (int i = 0; i < te->nDeps; i++) + { + DumpId depid = te->dependencies[i]; + + if (depid <= AH->maxDumpId && AH->tocsByDumpId[depid] != NULL) + { + TocEntry *otherte = AH->tocsByDumpId[depid]; + + if (strcmp(otherte->desc, "MATERIALIZED VIEW DATA") == 0) + return RESTORE_PASS_POST_ACL; + } + } + } + /* All else can be handled in the main pass. */ return RESTORE_PASS_MAIN; } @@ -4249,7 +4270,7 @@ restore_toc_entries_prefork(ArchiveHandle *AH, TocEntry *pending_list) * not set skipped_some in this case, since by assumption no main-pass * items could depend on these. */ - if (_tocEntryRestorePass(next_work_item) != RESTORE_PASS_MAIN) + if (_tocEntryRestorePass(AH, next_work_item) != RESTORE_PASS_MAIN) do_now = false; if (do_now) @@ -4331,7 +4352,7 @@ restore_toc_entries_parallel(ArchiveHandle *AH, ParallelState *pstate, * process in the current restore pass. */ AH->restorePass = RESTORE_PASS_MAIN; - move_to_ready_heap(pending_list, ready_heap, AH->restorePass); + move_to_ready_heap(AH, pending_list, ready_heap, AH->restorePass); /* * main parent loop @@ -4380,7 +4401,7 @@ restore_toc_entries_parallel(ArchiveHandle *AH, ParallelState *pstate, /* Advance to next restore pass */ AH->restorePass++; /* That probably allows some stuff to be made ready */ - move_to_ready_heap(pending_list, ready_heap, AH->restorePass); + move_to_ready_heap(AH, pending_list, ready_heap, AH->restorePass); /* Loop around to see if anything's now ready */ continue; } @@ -4551,7 +4572,8 @@ TocEntrySizeCompareBinaryheap(void *p1, void *p2, void *arg) * which applies the same logic one-at-a-time.) */ static void -move_to_ready_heap(TocEntry *pending_list, +move_to_ready_heap(ArchiveHandle *AH, + TocEntry *pending_list, binaryheap *ready_heap, RestorePass pass) { @@ -4564,7 +4586,7 @@ move_to_ready_heap(TocEntry *pending_list, next_te = te->pending_next; if (te->depCount == 0 && - _tocEntryRestorePass(te) == pass) + _tocEntryRestorePass(AH, te) == pass) { /* Remove it from pending_list ... */ pending_list_remove(te); @@ -4958,7 +4980,7 @@ reduce_dependencies(ArchiveHandle *AH, TocEntry *te, * memberships changed. */ if (otherte->depCount == 0 && - _tocEntryRestorePass(otherte) == AH->restorePass && + _tocEntryRestorePass(AH, otherte) == AH->restorePass && otherte->pending_prev != NULL && ready_heap != NULL) { diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index e41e645f649..84a78625820 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -3002,6 +3002,19 @@ makeTableDataInfo(DumpOptions *dopt, TableInfo *tbinfo) tbinfo->dataObj = tdinfo; + /* + * Materialized view statistics must be restored after the data, because + * REFRESH MATERIALIZED VIEW replaces the storage and resets the stats. + * + * The dependency is added here because the statistics objects are created + * first. + */ + if (tbinfo->relkind == RELKIND_MATVIEW && tbinfo->stats != NULL) + { + tbinfo->stats->section = SECTION_POST_DATA; + addObjectDependency(&tbinfo->stats->dobj, tdinfo->dobj.dumpId); + } + /* Make sure that we'll collect per-column info for this table. */ tbinfo->interesting = true; } @@ -6893,7 +6906,32 @@ getRelationStatistics(Archive *fout, DumpableObject *rel, int32 relpages, info->relkind = relkind; info->indAttNames = indAttNames; info->nindAttNames = nindAttNames; - info->postponed_def = false; + + /* + * Ordinarily, stats go in SECTION_DATA for tables and + * SECTION_POST_DATA for indexes. + * + * However, the section may be updated later for materialized view + * stats. REFRESH MATERIALIZED VIEW replaces the storage and resets + * the stats, so the stats must be restored after the data. Also, the + * materialized view definition may be postponed to SECTION_POST_DATA + * (see repairMatViewBoundaryMultiLoop()). + */ + switch (info->relkind) + { + case RELKIND_RELATION: + case RELKIND_PARTITIONED_TABLE: + case RELKIND_MATVIEW: + info->section = SECTION_DATA; + break; + case RELKIND_INDEX: + case RELKIND_PARTITIONED_INDEX: + info->section = SECTION_POST_DATA; + break; + default: + pg_fatal("cannot dump statistics for relation kind '%c'", + info->relkind); + } return info; } @@ -7292,9 +7330,17 @@ getTables(Archive *fout, int *numTables) /* Add statistics */ if (tblinfo[i].interesting) - getRelationStatistics(fout, &tblinfo[i].dobj, tblinfo[i].relpages, - PQgetvalue(res, i, i_reltuples), - relallvisible, tblinfo[i].relkind, NULL, 0); + { + RelStatsInfo *stats; + + stats = getRelationStatistics(fout, &tblinfo[i].dobj, + tblinfo[i].relpages, + PQgetvalue(res, i, i_reltuples), + relallvisible, + tblinfo[i].relkind, NULL, 0); + if (tblinfo[i].relkind == RELKIND_MATVIEW) + tblinfo[i].stats = stats; + } /* * Read-lock target tables to make sure they aren't DROPPED or altered @@ -10491,34 +10537,6 @@ appendNamedArgument(PQExpBuffer out, Archive *fout, const char *argname, appendPQExpBuffer(out, "::%s", argtype); } -/* - * Decide which section to use based on the relkind of the parent object. - * - * NB: materialized views may be postponed from SECTION_PRE_DATA to - * SECTION_POST_DATA to resolve some kinds of dependency problems. If so, the - * matview stats will also be postponed to SECTION_POST_DATA. See - * repairMatViewBoundaryMultiLoop(). - */ -static teSection -statisticsDumpSection(const RelStatsInfo *rsinfo) -{ - switch (rsinfo->relkind) - { - case RELKIND_RELATION: - case RELKIND_PARTITIONED_TABLE: - case RELKIND_MATVIEW: - return SECTION_DATA; - case RELKIND_INDEX: - case RELKIND_PARTITIONED_INDEX: - return SECTION_POST_DATA; - default: - pg_fatal("cannot dump statistics for relation kind '%c'", - rsinfo->relkind); - } - - return 0; /* keep compiler quiet */ -} - /* * dumpRelationStats -- * @@ -10531,8 +10549,6 @@ dumpRelationStats(Archive *fout, const RelStatsInfo *rsinfo) PGresult *res; PQExpBuffer query; PQExpBuffer out; - DumpId *deps = NULL; - int ndeps = 0; int i_attname; int i_inherited; int i_null_frac; @@ -10553,13 +10569,6 @@ dumpRelationStats(Archive *fout, const RelStatsInfo *rsinfo) if (!fout->dopt->dumpStatistics) return; - /* dependent on the relation definition, if doing schema */ - if (fout->dopt->dumpSchema) - { - deps = dobj->dependencies; - ndeps = dobj->nDeps; - } - query = createPQExpBuffer(); if (!fout->is_prepared[PREPQUERY_GETATTRIBUTESTATS]) { @@ -10737,11 +10746,10 @@ dumpRelationStats(Archive *fout, const RelStatsInfo *rsinfo) ARCHIVE_OPTS(.tag = dobj->name, .namespace = dobj->namespace->dobj.name, .description = "STATISTICS DATA", - .section = rsinfo->postponed_def ? - SECTION_POST_DATA : statisticsDumpSection(rsinfo), + .section = rsinfo->section, .createStmt = out->data, - .deps = deps, - .nDeps = ndeps)); + .deps = dobj->dependencies, + .nDeps = dobj->nDeps)); destroyPQExpBuffer(out); destroyPQExpBuffer(query); @@ -19429,7 +19437,7 @@ addBoundaryDependencies(DumpableObject **dobjs, int numObjs, break; case DO_REL_STATS: /* stats section varies by parent object type, DATA or POST */ - if (statisticsDumpSection((RelStatsInfo *) dobj) == SECTION_DATA) + if (((RelStatsInfo *) dobj)->section == SECTION_DATA) { addObjectDependency(dobj, preDataBound->dumpId); addObjectDependency(postDataBound, dobj->dumpId); diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h index bbdb30b5f54..70f7a369e4a 100644 --- a/src/bin/pg_dump/pg_dump.h +++ b/src/bin/pg_dump/pg_dump.h @@ -369,6 +369,7 @@ typedef struct _tableInfo bool *notnull_islocal; /* true if NOT NULL has local definition */ struct _attrDefInfo **attrdefs; /* DEFAULT expressions */ struct _constraintInfo *checkexprs; /* CHECK constraints */ + struct _relStatsInfo *stats; /* only set for matviews */ bool needs_override; /* has GENERATED ALWAYS AS IDENTITY */ char *amname; /* relation access method */ @@ -449,7 +450,7 @@ typedef struct _relStatsInfo */ char **indAttNames; /* attnames of the index, in order */ int32 nindAttNames; /* number of attnames stored (can be 0) */ - bool postponed_def; /* stats must be postponed into post-data */ + teSection section; /* stats may appear in data or post-data */ } RelStatsInfo; typedef struct _statsExtInfo diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c index f75e9928bad..0b0977788f1 100644 --- a/src/bin/pg_dump/pg_dump_sort.c +++ b/src/bin/pg_dump/pg_dump_sort.c @@ -820,7 +820,7 @@ repairMatViewBoundaryMultiLoop(DumpableObject *boundaryobj, RelStatsInfo *nextinfo = (RelStatsInfo *) nextobj; if (nextinfo->relkind == RELKIND_MATVIEW) - nextinfo->postponed_def = true; + nextinfo->section = SECTION_POST_DATA; } }