On Wed, May 25, 2016 at 4:21 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Magnus Hagander <mag...@hagander.net> writes: > >> Also, why didn't you keep using ExecuteSqlQueryForSingleRow()? > > > The reason I did that is that ExecuteSqlQueryForSingleRow() is a static > > method in pg_dump.c. I was planning to go back and review that, and > > consider moving it, but I forgot it :S > > > I think the clean thing is probably to use that one, and also move it > over > > to not be a static method in pg_dump.c, but instead sit next to > > ExecuteSqlQuery in pg_backup_db.c. Do you agree that's reasonable, and > > something that's OK to backpatch? > > No objection here, since it wouldn't be exposed outside pg_dump in any > case. > Here's an updated patch based on this,and the other feedback. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h index 83f6029..f94caa3 100644 --- a/src/bin/pg_dump/pg_backup.h +++ b/src/bin/pg_dump/pg_backup.h @@ -173,6 +173,7 @@ typedef struct Archive int verbose; char *remoteVersionStr; /* server's version string */ int remoteVersion; /* same in numeric form */ + bool isStandby; /* is server a standby node */ int minRemoteVersion; /* allowable range */ int maxRemoteVersion; diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c index 35ce945..818bc9e 100644 --- a/src/bin/pg_dump/pg_backup_db.c +++ b/src/bin/pg_dump/pg_backup_db.c @@ -37,6 +37,7 @@ _check_database_version(ArchiveHandle *AH) { const char *remoteversion_str; int remoteversion; + PGresult *res; remoteversion_str = PQparameterStatus(AH->connection, "server_version"); remoteversion = PQserverVersion(AH->connection); @@ -56,6 +57,20 @@ _check_database_version(ArchiveHandle *AH) remoteversion_str, progname, PG_VERSION); exit_horribly(NULL, "aborting because of server version mismatch\n"); } + + /* + * When running against 9.0 or later, check if we are in recovery mode, + * which means we are on a hot standby. + */ + if (remoteversion >= 90000) + { + res = ExecuteSqlQueryForSingleRow((Archive *) AH, "SELECT pg_catalog.pg_is_in_recovery()"); + + AH->public.isStandby = (strcmp(PQgetvalue(res, 0, 0), "t") == 0); + PQclear(res); + } + else + AH->public.isStandby = false; } /* @@ -389,6 +404,29 @@ ExecuteSqlQuery(Archive *AHX, const char *query, ExecStatusType status) } /* + * Execute an SQL query and verify that we got exactly one row back. + */ +PGresult * +ExecuteSqlQueryForSingleRow(Archive *fout, char *query) +{ + PGresult *res; + int ntups; + + res = ExecuteSqlQuery(fout, query, PGRES_TUPLES_OK); + + /* Expecting a single result only */ + ntups = PQntuples(res); + if (ntups != 1) + exit_horribly(NULL, + ngettext("query returned %d row instead of one: %s\n", + "query returned %d rows instead of one: %s\n", + ntups), + ntups, query); + + return res; +} + +/* * Convenience function to send a query. * Monitors result to detect COPY statements */ diff --git a/src/bin/pg_dump/pg_backup_db.h b/src/bin/pg_dump/pg_backup_db.h index 6408f14..527449e 100644 --- a/src/bin/pg_dump/pg_backup_db.h +++ b/src/bin/pg_dump/pg_backup_db.h @@ -16,6 +16,7 @@ extern int ExecuteSqlCommandBuf(Archive *AHX, const char *buf, size_t bufLen); extern void ExecuteSqlStatement(Archive *AHX, const char *query); extern PGresult *ExecuteSqlQuery(Archive *AHX, const char *query, ExecStatusType status); +extern PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query); extern void EndDBCopyMode(Archive *AHX, const char *tocEntryTag); diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 1267afb..a6550ed 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -264,7 +264,6 @@ static bool nonemptyReloptions(const char *reloptions); static void appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions, const char *prefix, Archive *fout); static char *get_synchronized_snapshot(Archive *fout); -static PGresult *ExecuteSqlQueryForSingleRow(Archive *fout, char *query); static void setupDumpWorker(Archive *AHX); @@ -648,23 +647,11 @@ main(int argc, char **argv) dopt.no_security_labels = 1; /* - * When running against 9.0 or later, check if we are in recovery mode, - * which means we are on a hot standby. + * On hot standby slaves, never try to dump unlogged table data, since it + * will just throw an error. */ - if (fout->remoteVersion >= 90000) - { - PGresult *res = ExecuteSqlQueryForSingleRow(fout, "SELECT pg_catalog.pg_is_in_recovery()"); - - if (strcmp(PQgetvalue(res, 0, 0), "t") == 0) - { - /* - * On hot standby slaves, never try to dump unlogged table data, - * since it will just throw an error. - */ - dopt.no_unlogged_table_data = true; - } - PQclear(res); - } + if (fout->isStandby) + dopt.no_unlogged_table_data = true; /* Select the appropriate subquery to convert user IDs to names */ if (fout->remoteVersion >= 80100) @@ -1096,7 +1083,16 @@ setup_connection(Archive *AH, const char *dumpencoding, else if (AH->numWorkers > 1 && AH->remoteVersion >= 90200 && !dopt->no_synchronized_snapshots) + { + if (AH->isStandby) + exit_horribly(NULL, + "Synchronized snapshots are not supported on standby servers.\n" + "Run with --no-synchronized-snapshots instead if you do not need\n" + "synchronized snapshots.\n"); + + AH->sync_snapshot_id = get_synchronized_snapshot(AH); + } } static void @@ -17845,26 +17841,3 @@ appendReloptionsArrayAH(PQExpBuffer buffer, const char *reloptions, if (!res) write_msg(NULL, "WARNING: could not parse reloptions array\n"); } - -/* - * Execute an SQL query and verify that we got exactly one row back. - */ -static PGresult * -ExecuteSqlQueryForSingleRow(Archive *fout, char *query) -{ - PGresult *res; - int ntups; - - res = ExecuteSqlQuery(fout, query, PGRES_TUPLES_OK); - - /* Expecting a single result only */ - ntups = PQntuples(res); - if (ntups != 1) - exit_horribly(NULL, - ngettext("query returned %d row instead of one: %s\n", - "query returned %d rows instead of one: %s\n", - ntups), - ntups, query); - - return res; -}
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers