public inbox for [email protected]  
help / color / mirror / Atom feed
From: Rian McGuire <[email protected]>
To: [email protected]
Subject: [PATCH] pg_dump: Do not dump statistics for excluded tables
Date: Wed, 27 Dec 2023 15:44:09 +1100
Message-ID: <[email protected]> (raw)

Hi hackers,

I've attached a patch against master that addresses a small bug in pg_dump.

Previously, pg_dump would include CREATE STATISTICS statements for
tables that were excluded from the dump, causing reload to fail if any
excluded tables had extended statistics.

The patch skips the creation of the StatsExtInfo if the associated
table does not have the DUMP_COMPONENT_DEFINITION flag set. This is
similar to how getPublicationTables behaves if a table is excluded.

I've covered this with a regression test by altering one of the CREATE
STATISTICS examples to work with the existing 'exclude_test_table'
run. Without the fix, that causes the test to fail with:
# Failed test 'exclude_test_table: should not dump CREATE STATISTICS
extended_stats_no_options'
# at t/002_pg_dump.pl line 4934.

Regards,
Rian
From 0fe06338728981fa727e3e6b99247741deda75fb Mon Sep 17 00:00:00 2001
From: Rian McGuire <[email protected]>
Date: Wed, 27 Dec 2023 13:09:31 +1100
Subject: [PATCH] pg_dump: Do not dump statistics for excluded tables

Previously, pg_dump would include CREATE STATISTICS statements for
tables that were excluded from the dump, causing reload to fail.
---
 src/bin/pg_dump/pg_dump.c        | 43 +++++++++++++++++++++++---------
 src/bin/pg_dump/t/002_pg_dump.pl |  5 ++--
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8c0b5486b9..c67ed416e9 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7288,11 +7288,13 @@ getExtendedStatistics(Archive *fout)
 	int			ntups;
 	int			i_tableoid;
 	int			i_oid;
+	int			i_stxrelid;
 	int			i_stxname;
 	int			i_stxnamespace;
 	int			i_stxowner;
 	int			i_stattarget;
-	int			i;
+	int			i,
+				j;
 
 	/* Extended statistics were new in v10 */
 	if (fout->remoteVersion < 100000)
@@ -7301,11 +7303,11 @@ getExtendedStatistics(Archive *fout)
 	query = createPQExpBuffer();
 
 	if (fout->remoteVersion < 130000)
-		appendPQExpBufferStr(query, "SELECT tableoid, oid, stxname, "
+		appendPQExpBufferStr(query, "SELECT tableoid, oid, stxrelid, stxname, "
 							 "stxnamespace, stxowner, (-1) AS stxstattarget "
 							 "FROM pg_catalog.pg_statistic_ext");
 	else
-		appendPQExpBufferStr(query, "SELECT tableoid, oid, stxname, "
+		appendPQExpBufferStr(query, "SELECT tableoid, oid, stxrelid, stxname, "
 							 "stxnamespace, stxowner, stxstattarget "
 							 "FROM pg_catalog.pg_statistic_ext");
 
@@ -7315,27 +7317,44 @@ getExtendedStatistics(Archive *fout)
 
 	i_tableoid = PQfnumber(res, "tableoid");
 	i_oid = PQfnumber(res, "oid");
+	i_stxrelid = PQfnumber(res, "stxrelid");
 	i_stxname = PQfnumber(res, "stxname");
 	i_stxnamespace = PQfnumber(res, "stxnamespace");
 	i_stxowner = PQfnumber(res, "stxowner");
 	i_stattarget = PQfnumber(res, "stxstattarget");
 
+	/* this allocation may be more than we need */
 	statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
+	j = 0;
 
 	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 =
+		Oid			stxrelid;
+		TableInfo  *tbinfo;
+
+		/*
+		 * Only dump extended statistics if we're going to dump the definition
+		 * of the table that they're associated with.
+		 */
+		stxrelid = atoi(PQgetvalue(res, i, i_stxrelid));
+		tbinfo = findTableByOid(stxrelid);
+		if (tbinfo == NULL || !(tbinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
+			continue;
+
+		statsextinfo[j].dobj.objType = DO_STATSEXT;
+		statsextinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
+		statsextinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
+		AssignDumpId(&statsextinfo[j].dobj);
+		statsextinfo[j].dobj.name = pg_strdup(PQgetvalue(res, i, i_stxname));
+		statsextinfo[j].dobj.namespace =
 			findNamespace(atooid(PQgetvalue(res, i, i_stxnamespace)));
-		statsextinfo[i].rolname = getRoleName(PQgetvalue(res, i, i_stxowner));
-		statsextinfo[i].stattarget = atoi(PQgetvalue(res, i, i_stattarget));
+		statsextinfo[j].rolname = getRoleName(PQgetvalue(res, i, i_stxowner));
+		statsextinfo[j].stattarget = atoi(PQgetvalue(res, i, i_stattarget));
 
 		/* Decide whether we want to dump it */
-		selectDumpableObject(&(statsextinfo[i].dobj), fout);
+		selectDumpableObject(&(statsextinfo[j].dobj), fout);
+
+		j++;
 	}
 
 	PQclear(res);
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index eb3ec534b4..a671603cd2 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -3742,14 +3742,15 @@ my %tests = (
 	'CREATE STATISTICS extended_stats_no_options' => {
 		create_order => 97,
 		create_sql => 'CREATE STATISTICS dump_test.test_ext_stats_no_options
-							ON col1, col2 FROM dump_test.test_fifth_table',
+							ON col1, col2 FROM dump_test.test_table',
 		regexp => qr/^
-			\QCREATE STATISTICS dump_test.test_ext_stats_no_options ON col1, col2 FROM dump_test.test_fifth_table;\E
+			\QCREATE STATISTICS dump_test.test_ext_stats_no_options ON col1, col2 FROM dump_test.test_table;\E
 		    /xms,
 		like =>
 		  { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
 		unlike => {
 			exclude_dump_test_schema => 1,
+			exclude_test_table => 1,
 			only_dump_measurement => 1,
 		},
 	},
-- 
2.43.0



Attachments:

  [text/plain] v1-0001-pg_dump-Do-not-dump-statistics-for-excluded-tables.patch (4.6K, ../[email protected]/2-v1-0001-pg_dump-Do-not-dump-statistics-for-excluded-tables.patch)
  download | inline diff:
From 0fe06338728981fa727e3e6b99247741deda75fb Mon Sep 17 00:00:00 2001
From: Rian McGuire <[email protected]>
Date: Wed, 27 Dec 2023 13:09:31 +1100
Subject: [PATCH] pg_dump: Do not dump statistics for excluded tables

Previously, pg_dump would include CREATE STATISTICS statements for
tables that were excluded from the dump, causing reload to fail.
---
 src/bin/pg_dump/pg_dump.c        | 43 +++++++++++++++++++++++---------
 src/bin/pg_dump/t/002_pg_dump.pl |  5 ++--
 2 files changed, 34 insertions(+), 14 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 8c0b5486b9..c67ed416e9 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -7288,11 +7288,13 @@ getExtendedStatistics(Archive *fout)
 	int			ntups;
 	int			i_tableoid;
 	int			i_oid;
+	int			i_stxrelid;
 	int			i_stxname;
 	int			i_stxnamespace;
 	int			i_stxowner;
 	int			i_stattarget;
-	int			i;
+	int			i,
+				j;
 
 	/* Extended statistics were new in v10 */
 	if (fout->remoteVersion < 100000)
@@ -7301,11 +7303,11 @@ getExtendedStatistics(Archive *fout)
 	query = createPQExpBuffer();
 
 	if (fout->remoteVersion < 130000)
-		appendPQExpBufferStr(query, "SELECT tableoid, oid, stxname, "
+		appendPQExpBufferStr(query, "SELECT tableoid, oid, stxrelid, stxname, "
 							 "stxnamespace, stxowner, (-1) AS stxstattarget "
 							 "FROM pg_catalog.pg_statistic_ext");
 	else
-		appendPQExpBufferStr(query, "SELECT tableoid, oid, stxname, "
+		appendPQExpBufferStr(query, "SELECT tableoid, oid, stxrelid, stxname, "
 							 "stxnamespace, stxowner, stxstattarget "
 							 "FROM pg_catalog.pg_statistic_ext");
 
@@ -7315,27 +7317,44 @@ getExtendedStatistics(Archive *fout)
 
 	i_tableoid = PQfnumber(res, "tableoid");
 	i_oid = PQfnumber(res, "oid");
+	i_stxrelid = PQfnumber(res, "stxrelid");
 	i_stxname = PQfnumber(res, "stxname");
 	i_stxnamespace = PQfnumber(res, "stxnamespace");
 	i_stxowner = PQfnumber(res, "stxowner");
 	i_stattarget = PQfnumber(res, "stxstattarget");
 
+	/* this allocation may be more than we need */
 	statsextinfo = (StatsExtInfo *) pg_malloc(ntups * sizeof(StatsExtInfo));
+	j = 0;
 
 	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 =
+		Oid			stxrelid;
+		TableInfo  *tbinfo;
+
+		/*
+		 * Only dump extended statistics if we're going to dump the definition
+		 * of the table that they're associated with.
+		 */
+		stxrelid = atoi(PQgetvalue(res, i, i_stxrelid));
+		tbinfo = findTableByOid(stxrelid);
+		if (tbinfo == NULL || !(tbinfo->dobj.dump & DUMP_COMPONENT_DEFINITION))
+			continue;
+
+		statsextinfo[j].dobj.objType = DO_STATSEXT;
+		statsextinfo[j].dobj.catId.tableoid = atooid(PQgetvalue(res, i, i_tableoid));
+		statsextinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
+		AssignDumpId(&statsextinfo[j].dobj);
+		statsextinfo[j].dobj.name = pg_strdup(PQgetvalue(res, i, i_stxname));
+		statsextinfo[j].dobj.namespace =
 			findNamespace(atooid(PQgetvalue(res, i, i_stxnamespace)));
-		statsextinfo[i].rolname = getRoleName(PQgetvalue(res, i, i_stxowner));
-		statsextinfo[i].stattarget = atoi(PQgetvalue(res, i, i_stattarget));
+		statsextinfo[j].rolname = getRoleName(PQgetvalue(res, i, i_stxowner));
+		statsextinfo[j].stattarget = atoi(PQgetvalue(res, i, i_stattarget));
 
 		/* Decide whether we want to dump it */
-		selectDumpableObject(&(statsextinfo[i].dobj), fout);
+		selectDumpableObject(&(statsextinfo[j].dobj), fout);
+
+		j++;
 	}
 
 	PQclear(res);
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index eb3ec534b4..a671603cd2 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -3742,14 +3742,15 @@ my %tests = (
 	'CREATE STATISTICS extended_stats_no_options' => {
 		create_order => 97,
 		create_sql => 'CREATE STATISTICS dump_test.test_ext_stats_no_options
-							ON col1, col2 FROM dump_test.test_fifth_table',
+							ON col1, col2 FROM dump_test.test_table',
 		regexp => qr/^
-			\QCREATE STATISTICS dump_test.test_ext_stats_no_options ON col1, col2 FROM dump_test.test_fifth_table;\E
+			\QCREATE STATISTICS dump_test.test_ext_stats_no_options ON col1, col2 FROM dump_test.test_table;\E
 		    /xms,
 		like =>
 		  { %full_runs, %dump_test_schema_runs, section_post_data => 1, },
 		unlike => {
 			exclude_dump_test_schema => 1,
+			exclude_test_table => 1,
 			only_dump_measurement => 1,
 		},
 	},
-- 
2.43.0



view thread (2+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: [PATCH] pg_dump: Do not dump statistics for excluded tables
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox