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

Commit 1298fcc

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 7f5b136 commit 1298fcc

File tree

5 files changed

+68
-78
lines changed

5 files changed

+68
-78
lines changed

‎src/bin/pg_dump/common.c

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -259,7 +259,7 @@ getSchemaData(Archive *fout, int *numTablesPtr)
259259

260260
if (g_verbose)
261261
write_msg(NULL, "reading extended statistics\n");
262-
getExtendedStatistics(fout, tblinfo, numTables);
262+
getExtendedStatistics(fout);
263263

264264
if (g_verbose)
265265
write_msg(NULL, "reading constraints\n");

‎src/bin/pg_dump/pg_backup_archiver.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3365,6 +3365,7 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
33653365
strcmp(type, "FOREIGN TABLE") == 0 ||
33663366
strcmp(type, "TEXT SEARCH DICTIONARY") == 0 ||
33673367
strcmp(type, "TEXT SEARCH CONFIGURATION") == 0 ||
3368+
strcmp(type, "STATISTICS") == 0 ||
33683369
/* non-schema-specified objects */
33693370
strcmp(type, "DATABASE") == 0 ||
33703371
strcmp(type, "PROCEDURAL LANGUAGE") == 0 ||
@@ -3581,6 +3582,7 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
35813582
strcmp(te->desc, "TEXT SEARCH CONFIGURATION") == 0 ||
35823583
strcmp(te->desc, "FOREIGN DATA WRAPPER") == 0 ||
35833584
strcmp(te->desc, "SERVER") == 0 ||
3585+
strcmp(te->desc, "STATISTICS") == 0 ||
35843586
strcmp(te->desc, "PUBLICATION") == 0 ||
35853587
strcmp(te->desc, "SUBSCRIPTION") == 0)
35863588
{
@@ -3602,8 +3604,7 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
36023604
strcmp(te->desc, "TRIGGER") == 0 ||
36033605
strcmp(te->desc, "ROW SECURITY") == 0 ||
36043606
strcmp(te->desc, "POLICY") == 0 ||
3605-
strcmp(te->desc, "USER MAPPING") == 0 ||
3606-
strcmp(te->desc, "STATISTICS") == 0)
3607+
strcmp(te->desc, "USER MAPPING") == 0)
36073608
{
36083609
/* these object types don't have separate owners */
36093610
}

‎src/bin/pg_dump/pg_dump.c

Lines changed: 61 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -6677,99 +6677,71 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
66776677

66786678
/*
66796679
* getExtendedStatistics
6680-
* get information about extended statistics on a dumpable table
6681-
* or materialized view.
6680+
* get information about extended-statistics objects.
66826681
*
66836682
* Note: extended statistics data is not returned directly to the caller, but
66846683
* it does get entered into the DumpableObject tables.
66856684
*/
66866685
void
6687-
getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables)
6686+
getExtendedStatistics(Archive *fout)
66886687
{
6689-
int i,
6690-
j;
66916688
PQExpBuffer query;
66926689
PGresult *res;
66936690
StatsExtInfo *statsextinfo;
66946691
int ntups;
66956692
int i_tableoid;
66966693
int i_oid;
66976694
int i_stxname;
6698-
int i_stxdef;
6695+
int i_stxnamespace;
6696+
int i_rolname;
6697+
int i;
66996698

67006699
/* Extended statistics were new in v10 */
67016700
if (fout->remoteVersion < 100000)
67026701
return;
67036702

67046703
query = createPQExpBuffer();
67056704

6706-
for (i = 0; i < numTables; i++)
6707-
{
6708-
TableInfo *tbinfo = &tblinfo[i];
6709-
6710-
/*
6711-
* Only plain tables, materialized views, foreign tables and
6712-
* partitioned tables can have extended statistics.
6713-
*/
6714-
if (tbinfo->relkind != RELKIND_RELATION &&
6715-
tbinfo->relkind != RELKIND_MATVIEW &&
6716-
tbinfo->relkind != RELKIND_FOREIGN_TABLE &&
6717-
tbinfo->relkind != RELKIND_PARTITIONED_TABLE)
6718-
continue;
6719-
6720-
/*
6721-
* Ignore extended statistics of tables whose definitions are not to
6722-
* be dumped.
6723-
*/
6724-
if (!(tbinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
6725-
continue;
6726-
6727-
if (g_verbose)
6728-
write_msg(NULL, "reading extended statistics for table \"%s.%s\"\n",
6729-
tbinfo->dobj.namespace->dobj.name,
6730-
tbinfo->dobj.name);
6731-
6732-
/* Make sure we are in proper schema so stadef is right */
6733-
selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
6705+
/* Make sure we are in proper schema */
6706+
selectSourceSchema(fout, "pg_catalog");
67346707

6735-
resetPQExpBuffer(query);
6708+
appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
6709+
"stxnamespace, (%s stxowner) AS rolname "
6710+
"FROM pg_catalog.pg_statistic_ext",
6711+
username_subquery);
67366712

6737-
appendPQExpBuffer(query,
6738-
"SELECT "
6739-
"tableoid, "
6740-
"oid, "
6741-
"stxname, "
6742-
"pg_catalog.pg_get_statisticsobjdef(oid) AS stxdef "
6743-
"FROM pg_catalog.pg_statistic_ext "
6744-
"WHERE stxrelid = '%u' "
6745-
"ORDER BY stxname", tbinfo->dobj.catId.oid);
6713+
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
67466714

6747-
res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
6715+
ntups = PQntuples(res);
67486716

6749-
ntups = PQntuples(res);
6717+
i_tableoid = PQfnumber(res, "tableoid");
6718+
i_oid = PQfnumber(res, "oid");
6719+
i_stxname = PQfnumber(res, "stxname");
6720+
i_stxnamespace = PQfnumber(res, "stxnamespace");
6721+
i_rolname = PQfnumber(res, "rolname");
67506722

6751-
i_tableoid = PQfnumber(res, "tableoid");
6752-
i_oid = PQfnumber(res, "oid");
6753-
i_stxname = PQfnumber(res, "stxname");
6754-
i_stxdef = PQfnumber(res, "stxdef");
6723+
statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
67556724

6756-
statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
6725+
for (i = 0; i < ntups; i++)
6726+
{
6727+
statsextinfo[i].dobj.objType = DO_STATSEXT;
6728+
statsextinfo[i].dobj.catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
6729+
statsextinfo[i].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
6730+
AssignDumpId(&statsextinfo[i].dobj);
6731+
statsextinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_stxname));
6732+
statsextinfo[i].dobj.namespace =
6733+
findNamespace(fout,
6734+
atooid(PQgetvalue(res, i, i_stxnamespace)));
6735+
statsextinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
67576736

6758-
for (j = 0; j < ntups; j++)
6759-
{
6760-
statsextinfo[j].dobj.objType = DO_STATSEXT;
6761-
statsextinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_tableoid));
6762-
statsextinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_oid));
6763-
AssignDumpId(&statsextinfo[j].dobj);
6764-
statsextinfo[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_stxname));
6765-
statsextinfo[j].dobj.namespace = tbinfo->dobj.namespace;
6766-
statsextinfo[j].statsexttable = tbinfo;
6767-
statsextinfo[j].statsextdef = pg_strdup(PQgetvalue(res, j, i_stxdef));
6768-
}
6737+
/* Decide whether we want to dump it */
6738+
selectDumpableObject(&(statsextinfo[i].dobj), fout);
67696739

6770-
PQclear(res);
6740+
/* Stats objects do not currently have ACLs. */
6741+
statsextinfo[i].dobj.dump &= ~DUMP_COMPONENT_ACL;
67716742
}
67726743

6744+
PQclear(res);
67736745
destroyPQExpBuffer(query);
67746746
}
67756747

@@ -15980,35 +15952,51 @@ static void
1598015952
dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
1598115953
{
1598215954
DumpOptions *dopt = fout->dopt;
15983-
TableInfo *tbinfo = statsextinfo->statsexttable;
1598415955
PQExpBuffer q;
1598515956
PQExpBuffer delq;
1598615957
PQExpBuffer labelq;
15958+
PQExpBuffer query;
15959+
PGresult *res;
15960+
char *stxdef;
1598715961

15988-
if (dopt->dataOnly)
15962+
/* Skip if not to be dumped */
15963+
if (!statsextinfo->dobj.dump || dopt->dataOnly)
1598915964
return;
1599015965

1599115966
q = createPQExpBuffer();
1599215967
delq = createPQExpBuffer();
1599315968
labelq = createPQExpBuffer();
15969+
query = createPQExpBuffer();
15970+
15971+
/* Make sure we are in proper schema so references are qualified */
15972+
selectSourceSchema(fout, statsextinfo->dobj.namespace->dobj.name);
15973+
15974+
appendPQExpBuffer(query, "SELECT "
15975+
"pg_catalog.pg_get_statisticsobjdef('%u'::pg_catalog.oid)",
15976+
statsextinfo->dobj.catId.oid);
15977+
15978+
res = ExecuteSqlQueryForSingleRow(fout, query->data);
15979+
15980+
stxdef = PQgetvalue(res, 0, 0);
1599415981

1599515982
appendPQExpBuffer(labelq, "STATISTICS %s",
1599615983
fmtId(statsextinfo->dobj.name));
1599715984

15998-
appendPQExpBuffer(q, "%s;\n", statsextinfo->statsextdef);
15985+
/* Result of pg_get_statisticsobjdef is complete except for semicolon */
15986+
appendPQExpBuffer(q, "%s;\n", stxdef);
1599915987

1600015988
appendPQExpBuffer(delq, "DROP STATISTICS %s.",
16001-
fmtId(tbinfo->dobj.namespace->dobj.name));
15989+
fmtId(statsextinfo->dobj.namespace->dobj.name));
1600215990
appendPQExpBuffer(delq, "%s;\n",
1600315991
fmtId(statsextinfo->dobj.name));
1600415992

1600515993
if (statsextinfo->dobj.dump & DUMP_COMPONENT_DEFINITION)
1600615994
ArchiveEntry(fout, statsextinfo->dobj.catId,
1600715995
statsextinfo->dobj.dumpId,
1600815996
statsextinfo->dobj.name,
16009-
tbinfo->dobj.namespace->dobj.name,
15997+
statsextinfo->dobj.namespace->dobj.name,
1601015998
NULL,
16011-
tbinfo->rolname, false,
15999+
statsextinfo->rolname, false,
1601216000
"STATISTICS", SECTION_POST_DATA,
1601316001
q->data, delq->data, NULL,
1601416002
NULL, 0,
@@ -16017,14 +16005,16 @@ dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
1601716005
/* Dump Statistics Comments */
1601816006
if (statsextinfo->dobj.dump & DUMP_COMPONENT_COMMENT)
1601916007
dumpComment(fout, labelq->data,
16020-
tbinfo->dobj.namespace->dobj.name,
16021-
tbinfo->rolname,
16008+
statsextinfo->dobj.namespace->dobj.name,
16009+
statsextinfo->rolname,
1602216010
statsextinfo->dobj.catId, 0,
1602316011
statsextinfo->dobj.dumpId);
1602416012

16013+
PQclear(res);
1602516014
destroyPQExpBuffer(q);
1602616015
destroyPQExpBuffer(delq);
1602716016
destroyPQExpBuffer(labelq);
16017+
destroyPQExpBuffer(query);
1602816018
}
1602916019

1603016020
/*

‎src/bin/pg_dump/pg_dump.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -369,8 +369,7 @@ typedef struct _indxInfo
369369
typedef struct _statsExtInfo
370370
{
371371
DumpableObject dobj;
372-
TableInfo *statsexttable; /* link to table the stats ext is for */
373-
char *statsextdef;
372+
char *rolname; /* name of owner, or empty string */
374373
} StatsExtInfo;
375374

376375
typedef struct _ruleInfo
@@ -683,7 +682,7 @@ extern TableInfo *getTables(Archive *fout, int *numTables);
683682
extern void getOwnedSeqs(Archive *fout, TableInfo tblinfo[], int numTables);
684683
extern InhInfo *getInherits(Archive *fout, int *numInherits);
685684
extern void getIndexes(Archive *fout, TableInfo tblinfo[], int numTables);
686-
extern void getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables);
685+
extern void getExtendedStatistics(Archive *fout);
687686
extern void getConstraints(Archive *fout, TableInfo tblinfo[], int numTables);
688687
extern RuleInfo *getRules(Archive *fout, int *numRules);
689688
extern void getTriggers(Archive *fout, TableInfo tblinfo[], int numTables);

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

Lines changed: 1 addition & 1 deletion
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)