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

diff --git a/src/bin/pg_dump/common.c b/src/bin/pg_dump/common.c
index 47191be86ad73cf2c89057abff8545c433716a7f..099d57eef265c6d235ea24b8897feab8935d9228 100644 (file)
--- a/src/bin/pg_dump/common.c
+++ b/src/bin/pg_dump/common.c
@@ -259,7 +259,7 @@ getSchemaData(Archive *fout, int *numTablesPtr)
 
        if (g_verbose)
                write_msg(NULL, "reading extended statistics\n");
-       getExtendedStatistics(fout, tblinfo, numTables);
+       getExtendedStatistics(fout);
 
        if (g_verbose)
                write_msg(NULL, "reading constraints\n");

diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 03f46bcbeef45c33422d77934240b2ee3aad2c91..3b6bc59b5a9027eb0a4bba12d8e9d3d594237b59 100644 (file)
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -3365,6 +3365,7 @@ _getObjectDescription(PQExpBuffer buf, TocEntry *te, ArchiveHandle *AH)
                strcmp(type, "FOREIGN TABLE") == 0 ||
                strcmp(type, "TEXT SEARCH DICTIONARY") == 0 ||
                strcmp(type, "TEXT SEARCH CONFIGURATION") == 0 ||
+               strcmp(type, "STATISTICS") == 0 ||
        /* non-schema-specified objects */
                strcmp(type, "DATABASE") == 0 ||
                strcmp(type, "PROCEDURAL LANGUAGE") == 0 ||
@@ -3581,6 +3582,7 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
                        strcmp(te->desc, "TEXT SEARCH CONFIGURATION") == 0 ||
                        strcmp(te->desc, "FOREIGN DATA WRAPPER") == 0 ||
                        strcmp(te->desc, "SERVER") == 0 ||
+                       strcmp(te->desc, "STATISTICS") == 0 ||
                        strcmp(te->desc, "PUBLICATION") == 0 ||
                        strcmp(te->desc, "SUBSCRIPTION") == 0)
                {
@@ -3602,8 +3604,7 @@ _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData)
                                 strcmp(te->desc, "TRIGGER") == 0 ||
                                 strcmp(te->desc, "ROW SECURITY") == 0 ||
                                 strcmp(te->desc, "POLICY") == 0 ||
-                                strcmp(te->desc, "USER MAPPING") == 0 ||
-                                strcmp(te->desc, "STATISTICS") == 0)
+                                strcmp(te->desc, "USER MAPPING") == 0)
                {
                        /* these object types don't have separate owners */
                }

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index f8de0a1f9388658ba8f5f548258383b3c207383c..34156bfe4e5080f28772355d37fbb0d456960eaa 100644 (file)
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -6677,17 +6677,14 @@ getIndexes(Archive *fout, TableInfo tblinfo[], int numTables)
 
 /*
  * getExtendedStatistics
- *       get information about extended statistics on a dumpable table
- *       or materialized view.
+ *       get information about extended-statistics objects.
  *
  * Note: extended statistics data is not returned directly to the caller, but
  * it does get entered into the DumpableObject tables.
  */
 void
-getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables)
+getExtendedStatistics(Archive *fout)
 {
-       int                     i,
-                               j;
        PQExpBuffer query;
        PGresult   *res;
        StatsExtInfo *statsextinfo;
@@ -6695,7 +6692,9 @@ getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables)
        int                     i_tableoid;
        int                     i_oid;
        int                     i_stxname;
-       int                     i_stxdef;
+       int                     i_stxnamespace;
+       int                     i_rolname;
+       int                     i;
 
        /* Extended statistics were new in v10 */
        if (fout->remoteVersion < 100000)
@@ -6703,73 +6702,46 @@ getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables)
 
        query = createPQExpBuffer();
 
-       for (i = 0; i < numTables; i++)
-       {
-               TableInfo  *tbinfo = &tblinfo[i];
-
-               /*
-                * Only plain tables, materialized views, foreign tables and
-                * partitioned tables can have extended statistics.
-                */
-               if (tbinfo->relkind != RELKIND_RELATION &&
-                       tbinfo->relkind != RELKIND_MATVIEW &&
-                       tbinfo->relkind != RELKIND_FOREIGN_TABLE &&
-                       tbinfo->relkind != RELKIND_PARTITIONED_TABLE)
-                       continue;
-
-               /*
-                * Ignore extended statistics of tables whose definitions are not to
-                * be dumped.
-                */
-               if (!(tbinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
-                       continue;
-
-               if (g_verbose)
-                       write_msg(NULL, "reading extended statistics for table \"%s.%s\"\n",
-                                         tbinfo->dobj.namespace->dobj.name,
-                                         tbinfo->dobj.name);
-
-               /* Make sure we are in proper schema so stadef is right */
-               selectSourceSchema(fout, tbinfo->dobj.namespace->dobj.name);
+       /* Make sure we are in proper schema */
+       selectSourceSchema(fout, "pg_catalog");
 
-               resetPQExpBuffer(query);
+       appendPQExpBuffer(query, "SELECT tableoid, oid, stxname, "
+                                         "stxnamespace, (%s stxowner) AS rolname "
+                                         "FROM pg_catalog.pg_statistic_ext",
+                                         username_subquery);
 
-               appendPQExpBuffer(query,
-                                                 "SELECT "
-                                                 "tableoid, "
-                                                 "oid, "
-                                                 "stxname, "
-                                                 "pg_catalog.pg_get_statisticsobjdef(oid) AS stxdef "
-                                                 "FROM pg_catalog.pg_statistic_ext "
-                                                 "WHERE stxrelid = '%u' "
-                                                 "ORDER BY stxname", tbinfo->dobj.catId.oid);
+       res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
 
-               res = ExecuteSqlQuery(fout, query->data, PGRES_TUPLES_OK);
+       ntups = PQntuples(res);
 
-               ntups = PQntuples(res);
+       i_tableoid = PQfnumber(res, "tableoid");
+       i_oid = PQfnumber(res, "oid");
+       i_stxname = PQfnumber(res, "stxname");
+       i_stxnamespace = PQfnumber(res, "stxnamespace");
+       i_rolname = PQfnumber(res, "rolname");
 
-               i_tableoid = PQfnumber(res, "tableoid");
-               i_oid = PQfnumber(res, "oid");
-               i_stxname = PQfnumber(res, "stxname");
-               i_stxdef = PQfnumber(res, "stxdef");
+       statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
 
-               statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
+       for (i = 0; i < ntups; i++)
+       {
+               statsextinfo[i].dobj.objType = DO_STATSEXT;
+               statsextinfo[i].dobj.catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
+               statsextinfo[i].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
+               AssignDumpId(&statsextinfo[i].dobj);
+               statsextinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_stxname));
+               statsextinfo[i].dobj.namespace =
+                       findNamespace(fout,
+                                                 atooid(PQgetvalue(res, i, i_stxnamespace)));
+               statsextinfo[i].rolname = pg_strdup(PQgetvalue(res, i, i_rolname));
 
-               for (j = 0; j < ntups; j++)
-               {
-                       statsextinfo[j].dobj.objType = DO_STATSEXT;
-                       statsextinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, j, i_tableoid));
-                       statsextinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, j, i_oid));
-                       AssignDumpId(&statsextinfo[j].dobj);
-                       statsextinfo[j].dobj.name = pg_strdup(PQgetvalue(res, j, i_stxname));
-                       statsextinfo[j].dobj.namespace = tbinfo->dobj.namespace;
-                       statsextinfo[j].statsexttable = tbinfo;
-                       statsextinfo[j].statsextdef = pg_strdup(PQgetvalue(res, j, i_stxdef));
-               }
+               /* Decide whether we want to dump it */
+               selectDumpableObject(&(statsextinfo[i].dobj), fout);
 
-               PQclear(res);
+               /* Stats objects do not currently have ACLs. */
+               statsextinfo[i].dobj.dump &= ~DUMP_COMPONENT_ACL;
        }
 
+       PQclear(res);
        destroyPQExpBuffer(query);
 }
 
@@ -15980,25 +15952,41 @@ static void
 dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
 {
        DumpOptions *dopt = fout->dopt;
-       TableInfo  *tbinfo = statsextinfo->statsexttable;
        PQExpBuffer q;
        PQExpBuffer delq;
        PQExpBuffer labelq;
+       PQExpBuffer query;
+       PGresult   *res;
+       char       *stxdef;
 
-       if (dopt->dataOnly)
+       /* Skip if not to be dumped */
+       if (!statsextinfo->dobj.dump || dopt->dataOnly)
                return;
 
        q = createPQExpBuffer();
        delq = createPQExpBuffer();
        labelq = createPQExpBuffer();
+       query = createPQExpBuffer();
+
+       /* Make sure we are in proper schema so references are qualified */
+       selectSourceSchema(fout, statsextinfo->dobj.namespace->dobj.name);
+
+       appendPQExpBuffer(query, "SELECT "
+                                         "pg_catalog.pg_get_statisticsobjdef('%u'::pg_catalog.oid)",
+                                         statsextinfo->dobj.catId.oid);
+
+       res = ExecuteSqlQueryForSingleRow(fout, query->data);
+
+       stxdef = PQgetvalue(res, 0, 0);
 
        appendPQExpBuffer(labelq, "STATISTICS %s",
                                          fmtId(statsextinfo->dobj.name));
 
-       appendPQExpBuffer(q, "%s;\n", statsextinfo->statsextdef);
+       /* Result of pg_get_statisticsobjdef is complete except for semicolon */
+       appendPQExpBuffer(q, "%s;\n", stxdef);
 
        appendPQExpBuffer(delq, "DROP STATISTICS %s.",
-                                         fmtId(tbinfo->dobj.namespace->dobj.name));
+                                         fmtId(statsextinfo->dobj.namespace->dobj.name));
        appendPQExpBuffer(delq, "%s;\n",
                                          fmtId(statsextinfo->dobj.name));
 
@@ -16006,9 +15994,9 @@ dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
                ArchiveEntry(fout, statsextinfo->dobj.catId,
                                         statsextinfo->dobj.dumpId,
                                         statsextinfo->dobj.name,
-                                        tbinfo->dobj.namespace->dobj.name,
+                                        statsextinfo->dobj.namespace->dobj.name,
                                         NULL,
-                                        tbinfo->rolname, false,
+                                        statsextinfo->rolname, false,
                                         "STATISTICS", SECTION_POST_DATA,
                                         q->data, delq->data, NULL,
                                         NULL, 0,
@@ -16017,14 +16005,16 @@ dumpStatisticsExt(Archive *fout, StatsExtInfo *statsextinfo)
        /* Dump Statistics Comments */
        if (statsextinfo->dobj.dump & DUMP_COMPONENT_COMMENT)
                dumpComment(fout, labelq->data,
-                                       tbinfo->dobj.namespace->dobj.name,
-                                       tbinfo->rolname,
+                                       statsextinfo->dobj.namespace->dobj.name,
+                                       statsextinfo->rolname,
                                        statsextinfo->dobj.catId, 0,
                                        statsextinfo->dobj.dumpId);
 
+       PQclear(res);
        destroyPQExpBuffer(q);
        destroyPQExpBuffer(delq);
        destroyPQExpBuffer(labelq);
+       destroyPQExpBuffer(query);
 }
 
 /*

diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index e7593e6da7fc324bfd16a264ebe92f70982dfc4c..941080cc753331dd99cb4ca7c454edeeb043de7b 100644 (file)
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -369,8 +369,7 @@ typedef struct _indxInfo
 typedef struct _statsExtInfo
 {
        DumpableObject dobj;
-       TableInfo  *statsexttable;      /* link to table the stats ext is for */
-       char       *statsextdef;
+       char       *rolname;            /* name of owner, or empty string */
 } StatsExtInfo;
 
 typedef struct _ruleInfo
@@ -683,7 +682,7 @@ extern TableInfo *getTables(Archive *fout, int *numTables);
 extern void getOwnedSeqs(Archive *fout, TableInfo tblinfo[], int numTables);
 extern InhInfo *getInherits(Archive *fout, int *numInherits);
 extern void getIndexes(Archive *fout, TableInfo tblinfo[], int numTables);
-extern void getExtendedStatistics(Archive *fout, TableInfo tblinfo[], int numTables);
+extern void getExtendedStatistics(Archive *fout);
 extern void getConstraints(Archive *fout, TableInfo tblinfo[], int numTables);
 extern RuleInfo *getRules(Archive *fout, int *numRules);
 extern void getTriggers(Archive *fout, TableInfo tblinfo[], int numTables);

diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index c492fbdc24d622d12dded3fd98667bfa6cd457f3..3f703d6a969b44dc0df7d75e51f9b4a4066b7b1c 100644 (file)
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -1420,7 +1420,7 @@ my %tests = (
        'ALTER ... OWNER commands (except post-data objects)' => {
                all_runs => 0,    # catch-all
                regexp =>
-qr/^ALTER (?!EVENT TRIGGER|LARGE OBJECT|PUBLICATION|SUBSCRIPTION)(.*) OWNER TO .*;/m,
+qr/^ALTER (?!EVENT TRIGGER|LARGE OBJECT|STATISTICS|PUBLICATION|SUBSCRIPTION)(.*) OWNER TO .*;/m,
                like   => {},     # use more-specific options above
                unlike => {
                        column_inserts           => 1,