pgsql: Make pg_dump exclude unlogged table data on hot standby slaves

Started by Magnus Haganderalmost 13 years ago6 messages
#1Magnus Hagander
magnus@hagander.net

Make pg_dump exclude unlogged table data on hot standby slaves

Noted by Joe Van Dyk

Branch
------
REL9_1_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/1cc43979cf44db0b3da77e34493689fe13484fa0

Modified Files
--------------
doc/src/sgml/ref/pg_dump.sgml | 3 ++-
src/bin/pg_dump/pg_dump.c | 18 ++++++++++++++++++
2 files changed, 20 insertions(+), 1 deletions(-)

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#2Andres Freund
andres@2ndquadrant.com
In reply to: Magnus Hagander (#1)
1 attachment(s)
Re: pgsql: Make pg_dump exclude unlogged table data on hot standby slaves

On 2013-01-25 08:49:10 +0000, Magnus Hagander wrote:

Make pg_dump exclude unlogged table data on hot standby slaves

This missed the fact that there is no ExecuteSqlQueryForSingleRow and
surroundign infrastructure.

Fix attached.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

0001-Fix-backpatch-of-unlogged-table-check-for-pg_dump.patchtext/x-patch; charset=us-asciiDownload
>From c6a4ffe4c70a861c676a2447dfd3ab518d2e2f2b Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Fri, 25 Jan 2013 13:21:03 +0100
Subject: [PATCH] Fix backpatch of unlogged table check for pg_dump

The previous commit used ExecuteSqlQueryForSingleRow which is only there in
9.2+.
---
 src/bin/pg_dump/pg_dump.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bef2b43..964823f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -656,9 +656,25 @@ main(int argc, char **argv)
 	 * When running against 9.0 or later, check if we are in recovery mode,
 	 * which means we are on a hot standby.
 	 */
-	if (fout->remoteVersion >= 90000)
+	if (g_fout->remoteVersion >= 90000)
 	{
-		PGresult *res = ExecuteSqlQueryForSingleRow(fout, "SELECT pg_catalog.pg_is_in_recovery()");
+		PGresult *res;
+		const char *query = "SELECT pg_catalog.pg_is_in_recovery()";
+		int ntups;
+
+		res = PQexec(g_conn, query);
+		check_sql_result(res, g_conn, query, PGRES_TUPLES_OK);
+		ntups = PQntuples(res);
+
+		if (ntups != 1)
+		{
+			write_msg(NULL, ngettext("query returned %d row instead of one: %s\n",
+									 "query returned %d rows instead of one: %s\n",
+									 ntups),
+					  ntups, query);
+			exit_nicely();
+		}
+
 		if (strcmp(PQgetvalue(res, 0, 0), "t") == 0)
 		{
 			/*
-- 
1.7.12.289.g0ce9864.dirty

#3Magnus Hagander
magnus@hagander.net
In reply to: Andres Freund (#2)
Re: pgsql: Make pg_dump exclude unlogged table data on hot standby slaves

On Fri, Jan 25, 2013 at 1:31 PM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2013-01-25 08:49:10 +0000, Magnus Hagander wrote:

Make pg_dump exclude unlogged table data on hot standby slaves

This missed the fact that there is no ExecuteSqlQueryForSingleRow and
surroundign infrastructure.

Ugh, that's what I get for pushing something just before getting on a
plane. Thanks for spotting.

Fix looks good, except I'd just put the query text inline in PQexec()
and not bother with a separate variable..

Unfortunately, the wifi at this airport refuses to let me push through
ssh, and I don't really have the time to get the tunnels properly set
up before my next flight. If another commiter could pick that one up
and just push it (9.1 branch only), I'd appreciate it!

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#4Andres Freund
andres@2ndquadrant.com
In reply to: Magnus Hagander (#3)
1 attachment(s)
Re: [COMMITTERS] pgsql: Make pg_dump exclude unlogged table data on hot standby slaves

On 2013-01-25 13:56:11 +0100, Magnus Hagander wrote:

On Fri, Jan 25, 2013 at 1:31 PM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2013-01-25 08:49:10 +0000, Magnus Hagander wrote:

Make pg_dump exclude unlogged table data on hot standby slaves

This missed the fact that there is no ExecuteSqlQueryForSingleRow and
surroundign infrastructure.

Ugh, that's what I get for pushing something just before getting on a
plane. Thanks for spotting.

Fix looks good, except I'd just put the query text inline in PQexec()
and not bother with a separate variable..

The query needs to get passed to check_sql_result and write_msg as well,
thats why I added the extra variable.

I don't think my -committers post got through (seems to be restricted),
so I am CCing -hackers so somebody else can see the patch.

Andres

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

Attachments:

0001-Fix-backpatch-of-unlogged-table-check-for-pg_dump.patchtext/x-patch; charset=us-asciiDownload
>From c6a4ffe4c70a861c676a2447dfd3ab518d2e2f2b Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Fri, 25 Jan 2013 13:21:03 +0100
Subject: [PATCH] Fix backpatch of unlogged table check for pg_dump

The previous commit used ExecuteSqlQueryForSingleRow which is only there in
9.2+.
---
 src/bin/pg_dump/pg_dump.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index bef2b43..964823f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -656,9 +656,25 @@ main(int argc, char **argv)
 	 * When running against 9.0 or later, check if we are in recovery mode,
 	 * which means we are on a hot standby.
 	 */
-	if (fout->remoteVersion >= 90000)
+	if (g_fout->remoteVersion >= 90000)
 	{
-		PGresult *res = ExecuteSqlQueryForSingleRow(fout, "SELECT pg_catalog.pg_is_in_recovery()");
+		PGresult *res;
+		const char *query = "SELECT pg_catalog.pg_is_in_recovery()";
+		int ntups;
+
+		res = PQexec(g_conn, query);
+		check_sql_result(res, g_conn, query, PGRES_TUPLES_OK);
+		ntups = PQntuples(res);
+
+		if (ntups != 1)
+		{
+			write_msg(NULL, ngettext("query returned %d row instead of one: %s\n",
+									 "query returned %d rows instead of one: %s\n",
+									 ntups),
+					  ntups, query);
+			exit_nicely();
+		}
+
 		if (strcmp(PQgetvalue(res, 0, 0), "t") == 0)
 		{
 			/*
-- 
1.7.12.289.g0ce9864.dirty

#5Magnus Hagander
magnus@hagander.net
In reply to: Andres Freund (#4)
Re: pgsql: Make pg_dump exclude unlogged table data on hot standby slaves

On Fri, Jan 25, 2013 at 1:59 PM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2013-01-25 13:56:11 +0100, Magnus Hagander wrote:

On Fri, Jan 25, 2013 at 1:31 PM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2013-01-25 08:49:10 +0000, Magnus Hagander wrote:

Make pg_dump exclude unlogged table data on hot standby slaves

This missed the fact that there is no ExecuteSqlQueryForSingleRow and
surroundign infrastructure.

Ugh, that's what I get for pushing something just before getting on a
plane. Thanks for spotting.

Fix looks good, except I'd just put the query text inline in PQexec()
and not bother with a separate variable..

The query needs to get passed to check_sql_result and write_msg as well,
thats why I added the extra variable.

Ahh, gotcha.

I don't think my -committers post got through (seems to be restricted),
so I am CCing -hackers so somebody else can see the patch.

Ok! I just talked to Robert, and he's taking a look at it for me.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

#6Robert Haas
robertmhaas@gmail.com
In reply to: Magnus Hagander (#5)
Re: pgsql: Make pg_dump exclude unlogged table data on hot standby slaves

On Fri, Jan 25, 2013 at 8:01 AM, Magnus Hagander <magnus@hagander.net> wrote:

On Fri, Jan 25, 2013 at 1:59 PM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2013-01-25 13:56:11 +0100, Magnus Hagander wrote:

On Fri, Jan 25, 2013 at 1:31 PM, Andres Freund <andres@2ndquadrant.com> wrote:

On 2013-01-25 08:49:10 +0000, Magnus Hagander wrote:

Make pg_dump exclude unlogged table data on hot standby slaves

This missed the fact that there is no ExecuteSqlQueryForSingleRow and
surroundign infrastructure.

Ugh, that's what I get for pushing something just before getting on a
plane. Thanks for spotting.

Fix looks good, except I'd just put the query text inline in PQexec()
and not bother with a separate variable..

The query needs to get passed to check_sql_result and write_msg as well,
thats why I added the extra variable.

Ahh, gotcha.

I don't think my -committers post got through (seems to be restricted),
so I am CCing -hackers so somebody else can see the patch.

Ok! I just talked to Robert, and he's taking a look at it for me.

Committed.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers