Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 5c9f256

Browse files
tglsfdctglsfdc
committed
Fix assorted errors in pg_dump's handling of extended statistics objects.
pg_dump supposed that a stats object necessarily shares the same schema as its underlying table, and that it doesn't have a separate owner. These things may have been true during early development of the feature, but they are not true as of v10 release. Failure to track the object's schema separately turns out to have only limited consequences, because pg_get_statisticsobjdef() always schema- qualifies the target object name in the generated CREATE STATISTICS command (a decision out of step with the rest of ruleutils.c, but I digress). Therefore the restored object would be in the right schema, so that the only problem is that the TOC entry would be mislabeled as to schema. That could lead to wrong decisions for schema-selective restores, for example. The ownership issue is a bit more serious: not only was the TOC entry potentially mislabeled as to owner, but pg_dump didn't bother to issue an ALTER OWNER command at all, so that after restore the stats object would continue to be owned by the restoring superuser. A final point is that decisions as to whether to dump a stats object or not were driven by whether the underlying table was dumped or not. While that's not wrong on its face, it won't scale nicely to the planned future extension to cross-table statistics. Moreover, that design decision comes out of the view of stats objects as being auxiliary to a particular table, like a rule or trigger, which is exactly where the above problems came from. Since we're now treating stats objects more like independent objects in their own right, they ought to behave like standalone objects for this purpose too. So change to using the generic selectDumpableObject() logic for them (which presently amounts to "dump if containing schema is to be dumped"). Along the way to fixing this, restructure so that getExtendedStatistics collects the identity info (only) for all extended stats objects in one query, and then for each object actually being dumped, we retrieve the definition in dumpStatisticsExt. This is necessary to ensure that schema-qualification in the generated CREATE STATISTICS command happens with respect to the search path that pg_dump will now be using at restore time (ie, the schema the stats object is in, not that of the underlying table). It's probably also significantly faster in the typical scenario where only a minority of tables have extended stats. Back-patch to v10 where extended stats were introduced. Discussion: https://postgr.es/m/18272.1518328606@sss.pgh.pa.us
1 parent d02d4a6 commit 5c9f256

File tree

5 files changed

+68
-78
lines changed

5 files changed

+68
-78
lines changed

‎src/bin/pg_dump/common.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -266,7 +266,7 @@ getSchemaData(Archive *fout, int *numTablesPtr)
266266

267267
if (g_verbose)
268268
write_msg(NULL, "reading extended statistics\n");
269-
getExtendedStatistics(fout, tblinfo, numTables);
269+
getExtendedStatistics(fout);
270270

271271
if (g_verbose)
272272
write_msg(NULL, "reading constraints\n");

‎src/bin/pg_dump/pg_backup_archiver.c

+3-2
Original file line numberDiff line numberDiff line change
@@ -3441,6 +3441,7 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
34413441
strcmp(type, "FOREIGN TABLE") == 0 ||
34423442
strcmp(type, "TEXT SEARCH DICTIONARY") == 0 ||
34433443
strcmp(type, "TEXT SEARCH CONFIGURATION") == 0 ||
3444+
strcmp(type, "STATISTICS") == 0 ||
34443445
/* non-schema-specified objects */
34453446
strcmp(type, "DATABASE") == 0 ||
34463447
strcmp(type, "PROCEDURAL LANGUAGE") == 0 ||
@@ -3636,6 +3637,7 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
36363637
strcmp(te->desc, "TEXT SEARCH CONFIGURATION") == 0 ||
36373638
strcmp(te->desc, "FOREIGN DATA WRAPPER") == 0 ||
36383639
strcmp(te->desc, "SERVER") == 0 ||
3640+
strcmp(te->desc, "STATISTICS") == 0 ||
36393641
strcmp(te->desc, "PUBLICATION") == 0 ||
36403642
strcmp(te->desc, "SUBSCRIPTION") == 0)
36413643
{
@@ -3658,8 +3660,7 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
36583660
strcmp(te->desc, "TRIGGER") == 0 ||
36593661
strcmp(te->desc, "ROW SECURITY") == 0 ||
36603662
strcmp(te->desc, "POLICY") == 0 ||
3661-
strcmp(te->desc, "USER MAPPING") == 0 ||
3662-
strcmp(te->desc, "STATISTICS") == 0)
3663+
strcmp(te->desc, "USER MAPPING") == 0)
36633664
{
36643665
/* these object types don't have separate owners */
36653666
}

‎src/bin/pg_dump/pg_dump.c

+61-71
Original file line numberDiff line numberDiff line change
@@ -7020,99 +7020,71 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
70207020

70217021
/*
70227022
* getExtendedStatistics
7023-
* get information about extended statistics on a dumpable table
7024-
* or materialized view.
7023+
* get information about extended-statistics objects.
70257024
*
70267025
* Note: extended statistics data is not returned directly to the caller, but
70277026
* it does get entered into the DumpableObject tables.
70287027
*/
70297028
void
7030-
getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables)
7029+
getExtendedStatistics(Archive *fout)
70317030
{
7032-
int i,
7033-
j;
70347031
PQExpBuffer query;
70357032
PGresult *res;
70367033
StatsExtInfo *statsextinfo;
70377034
int ntups;
70387035
int i_tableoid;
70397036
int i_oid;
70407037
int i_stxname;
7041-
int i_stxdef;
7038+
int i_stxnamespace;
7039+
int i_rolname;
7040+
int i;
70427041

70437042
/* Extended statistics were new in v10 */
70447043
if (fout->remoteVersion < 100000)
70457044
return;
70467045

70477046
query = createPQExpBuffer();
70487047

7049-
for (i = 0; i < numTables; i++)
7050-
{
7051-
TableInfo *tbinfo = &tblinfo[i];
7052-
7053-
/*
7054-
* Only plain tables, materialized views, foreign tables and
7055-
* partitioned tables can have extended statistics.
7056-
*/
7057-
if (tbinfo->relkind != RELKIND_RELATION &&
7058-
tbinfo->relkind != RELKIND_MATVIEW &&
7059-
tbinfo->relkind != RELKIND_FOREIGN_TABLE &&
7060-
tbinfo->relkind != RELKIND_PARTITIONED_TABLE)
7061-
continue;
7062-
7063-
/*
7064-
* Ignore extended statistics of tables whose definitions are not to
7065-
* be dumped.
7066-
*/
7067-
if (!(tbinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
7068-
continue;
7069-
7070-
if (g_verbose)
7071-
write_msg(NULL, "reading extended statistics for table \"%s.%s\"\n",
7072-
tbinfo->dobj.namespace->dobj.name,
7073-
tbinfo->dobj.name);
7074-
7075-
/* Make sure we are in proper schema so stadef is right */
7076-
selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
7048+
/* Make sure we are in proper schema */
7049+
selectSourceSchema(fout, "pg_catalog");
70777050

7078-
resetPQExpBuffer(query);
7051+
appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
7052+
"stxnamespace, (%s stxowner) AS rolname "
7053+
"FROM pg_catalog.pg_statistic_ext",
7054+
username_subquery);
70797055

7080-
appendPQExpBuffer(query,
7081-
"SELECT "
7082-
"tableoid, "
7083-
"oid, "
7084-
"stxname, "
7085-
"pg_catalog.pg_get_statisticsobjdef(oid) AS stxdef "
7086-
"FROM pg_catalog.pg_statistic_ext "
7087-
"WHERE stxrelid = '%u' "
7088-
"ORDER BY stxname", tbinfo->dobj.catId.oid);
7056+
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
70897057

7090-
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
7058+
ntups = PQntuples(res);
70917059

7092-
ntups = PQntuples(res);
7060+
i_tableoid = PQfnumber(res, "tableoid");
7061+
i_oid = PQfnumber(res, "oid");
7062+
i_stxname = PQfnumber(res, "stxname");
7063+
i_stxnamespace = PQfnumber(res, "stxnamespace");
7064+
i_rolname = PQfnumber(res, "rolname");
70937065

7094-
i_tableoid = PQfnumber(res, "tableoid");
7095-
i_oid = PQfnumber(res, "oid");
7096-
i_stxname = PQfnumber(res, "stxname");
7097-
i_stxdef = PQfnumber(res, "stxdef");
7066+
statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
70987067

7099-
statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
7068+
for (i = 0; i < ntups; i++)
7069+
{
7070+
statsextinfo[i].dobj.objType = DO_STATSEXT;
7071+
statsextinfo[i].dobj.catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
7072+
statsextinfo[i].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
7073+
AssignDumpId(&statsextinfo[i].dobj);
7074+
statsextinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_stxname));
7075+
statsextinfo[i].dobj.namespace =
7076+
findNamespace(fout,
7077+
atooid(PQgetvalue(res, i, i_stxnamespace)));
7078+
statsextinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
71007079

7101-
for (j = 0; j < ntups; j++)
7102-
{
7103-
statsextinfo[j].dobj.objType = DO_STATSEXT;
7104-
statsextinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_tableoid));
7105-
statsextinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_oid));
7106-
AssignDumpId(&statsextinfo[j].dobj);
7107-
statsextinfo[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_stxname));
7108-
statsextinfo[j].dobj.namespace = tbinfo->dobj.namespace;
7109-
statsextinfo[j].statsexttable = tbinfo;
7110-
statsextinfo[j].statsextdef = pg_strdup(PQgetvalue(res, j, i_stxdef));
7111-
}
7080+
/* Decide whether we want to dump it */
7081+
selectDumpableObject(&(statsextinfo[i].dobj), fout);
71127082

7113-
PQclear(res);
7083+
/* Stats objects do not currently have ACLs. */
7084+
statsextinfo[i].dobj.dump &= ~DUMP_COMPONENT_ACL;
71147085
}
71157086

7087+
PQclear(res);
71167088
destroyPQExpBuffer(query);
71177089
}
71187090

@@ -16486,35 +16458,51 @@ static void
1648616458
dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
1648716459
{
1648816460
DumpOptions *dopt = fout->dopt;
16489-
TableInfo *tbinfo = statsextinfo->statsexttable;
1649016461
PQExpBuffer q;
1649116462
PQExpBuffer delq;
1649216463
PQExpBuffer labelq;
16464+
PQExpBuffer query;
16465+
PGresult *res;
16466+
char *stxdef;
1649316467

16494-
if (dopt->dataOnly)
16468+
/* Skip if not to be dumped */
16469+
if (!statsextinfo->dobj.dump || dopt->dataOnly)
1649516470
return;
1649616471

1649716472
q = createPQExpBuffer();
1649816473
delq = createPQExpBuffer();
1649916474
labelq = createPQExpBuffer();
16475+
query = createPQExpBuffer();
16476+
16477+
/* Make sure we are in proper schema so references are qualified */
16478+
selectSourceSchema(fout, statsextinfo->dobj.namespace->dobj.name);
16479+
16480+
appendPQExpBuffer(query, "SELECT "
16481+
"pg_catalog.pg_get_statisticsobjdef('%u'::pg_catalog.oid)",
16482+
statsextinfo->dobj.catId.oid);
16483+
16484+
res = ExecuteSqlQueryForSingleRow(fout, query->data);
16485+
16486+
stxdef = PQgetvalue(res, 0, 0);
1650016487

1650116488
appendPQExpBuffer(labelq, "STATISTICS %s",
1650216489
fmtId(statsextinfo->dobj.name));
1650316490

16504-
appendPQExpBuffer(q, "%s;\n", statsextinfo->statsextdef);
16491+
/* Result of pg_get_statisticsobjdef is complete except for semicolon */
16492+
appendPQExpBuffer(q, "%s;\n", stxdef);
1650516493

1650616494
appendPQExpBuffer(delq, "DROP STATISTICS %s.",
16507-
fmtId(tbinfo->dobj.namespace->dobj.name));
16495+
fmtId(statsextinfo->dobj.namespace->dobj.name));
1650816496
appendPQExpBuffer(delq, "%s;\n",
1650916497
fmtId(statsextinfo->dobj.name));
1651016498

1651116499
if (statsextinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
1651216500
ArchiveEntry(fout, statsextinfo->dobj.catId,
1651316501
statsextinfo->dobj.dumpId,
1651416502
statsextinfo->dobj.name,
16515-
tbinfo->dobj.namespace->dobj.name,
16503+
statsextinfo->dobj.namespace->dobj.name,
1651616504
NULL,
16517-
tbinfo->rolname, false,
16505+
statsextinfo->rolname, false,
1651816506
"STATISTICS", SECTION_POST_DATA,
1651916507
q->data, delq->data, NULL,
1652016508
NULL, 0,
@@ -16523,14 +16511,16 @@ dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
1652316511
/* Dump Statistics Comments */
1652416512
if (statsextinfo->dobj.dump & DUMP_COMPONENT_COMMENT)
1652516513
dumpComment(fout, labelq->data,
16526-
tbinfo->dobj.namespace->dobj.name,
16527-
tbinfo->rolname,
16514+
statsextinfo->dobj.namespace->dobj.name,
16515+
statsextinfo->rolname,
1652816516
statsextinfo->dobj.catId, 0,
1652916517
statsextinfo->dobj.dumpId);
1653016518

16519+
PQclear(res);
1653116520
destroyPQExpBuffer(q);
1653216521
destroyPQExpBuffer(delq);
1653316522
destroyPQExpBuffer(labelq);
16523+
destroyPQExpBuffer(query);
1653416524
}
1653516525

1653616526
/*

‎src/bin/pg_dump/pg_dump.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -380,8 +380,7 @@ typedef struct _indexAttachInfo
380380
typedef struct _statsExtInfo
381381
{
382382
DumpableObject dobj;
383-
TableInfo *statsexttable; /* link to table the stats ext is for */
384-
char *statsextdef;
383+
char *rolname; /* name of owner, or empty string */
385384
} StatsExtInfo;
386385

387386
typedef struct _ruleInfo
@@ -694,7 +693,7 @@ extern TableInfo *getTables(Archive *fout, int *numTables);
694693
extern void getOwnedSeqs(Archive *fout, TableInfo tblinfo[], int numTables);
695694
extern InhInfo *getInherits(Archive *fout, int *numInherits);
696695
extern void getIndexes(Archive *fout, TableInfo tblinfo[], int numTables);
697-
extern void getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables);
696+
extern void getExtendedStatistics(Archive *fout);
698697
extern void getConstraints(Archive *fout, TableInfo tblinfo[], int numTables);
699698
extern RuleInfo *getRules(Archive *fout, int *numRules);
700699
extern void getTriggers(Archive *fout, TableInfo tblinfo[], int numTables);

‎src/bin/pg_dump/t/002_pg_dump.pl

+1-1
Original file line numberDiff line numberDiff line change
@@ -1420,7 +1420,7 @@
14201420
'ALTER ... OWNER commands (except post-data objects)' => {
14211421
all_runs => 0, # catch-all
14221422
regexp =>
1423-
qr/^ALTER (?!EVENT TRIGGER|LARGE OBJECT|PUBLICATION|SUBSCRIPTION)(.*) OWNER TO .*;/m,
1423+
qr/^ALTER (?!EVENT TRIGGER|LARGE OBJECT|STATISTICS|PUBLICATION|SUBSCRIPTION)(.*) OWNER TO .*;/m,
14241424
like => {}, # use more-specific options above
14251425
unlike => {
14261426
column_inserts => 1,

0 commit comments

Comments
 (0)