From 634f1e47c51121dc754f651e8e57d8bd5d7a077c Mon Sep 17 00:00:00 2001 From: Peter Smith Date: Tue, 13 Jul 2021 18:51:37 +1000 Subject: [PATCH v1] Avoid unnecessary calls to PGserverVersion. Some functions were found to be making repeated calls to PQserverVersion(conn), instead of just calling once and assigning the result to a local variable. This patch culls those extra calls. --- contrib/postgres_fdw/postgres_fdw.c | 6 ++++-- .../replication/libpqwalreceiver/libpqwalreceiver.c | 7 ++++--- src/bin/pg_basebackup/pg_basebackup.c | 15 ++++++++++----- src/bin/scripts/reindexdb.c | 6 ++++-- src/bin/scripts/vacuumdb.c | 20 +++++++++++--------- src/fe_utils/string_utils.c | 9 +++++---- 6 files changed, 38 insertions(+), 25 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index f15c97a..e0466c4 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -5191,6 +5191,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid) ForeignServer *server; UserMapping *mapping; PGconn *conn; + int server_version; StringInfoData buf; PGresult *volatile res = NULL; int numrows, @@ -5221,9 +5222,10 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid) server = GetForeignServer(serverOid); mapping = GetUserMapping(GetUserId(), server->serverid); conn = GetConnection(mapping, false, NULL); + server_version = PQserverVersion(conn); /* Don't attempt to import collation if remote server hasn't got it */ - if (PQserverVersion(conn) < 90100) + if (server_version < 90100) import_collate = false; /* Create workspace for strings */ @@ -5318,7 +5320,7 @@ postgresImportForeignSchema(ImportForeignSchemaStmt *stmt, Oid serverOid) deparseStringLiteral(&buf, stmt->remote_schema); /* Partitions are supported since Postgres 10 */ - if (PQserverVersion(conn) >= 100000 && + if (server_version >= 100000 && stmt->list_type != FDW_IMPORT_SCHEMA_LIMIT_TO) appendStringInfoString(&buf, " AND NOT c.relispartition "); diff --git a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c index 19ea159..111aba0 100644 --- a/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c +++ b/src/backend/replication/libpqwalreceiver/libpqwalreceiver.c @@ -427,6 +427,7 @@ libpqrcv_startstreaming(WalReceiverConn *conn, char *pubnames_str; List *pubnames; char *pubnames_literal; + int server_version = PQserverVersion(conn->streamConn); appendStringInfoString(&cmd, " ("); @@ -434,11 +435,11 @@ libpqrcv_startstreaming(WalReceiverConn *conn, options->proto.logical.proto_version); if (options->proto.logical.streaming && - PQserverVersion(conn->streamConn) >= 140000) + server_version >= 140000) appendStringInfoString(&cmd, ", streaming 'on'"); if (options->proto.logical.twophase && - PQserverVersion(conn->streamConn) >= 150000) + server_version >= 150000) appendStringInfoString(&cmd, ", two_phase 'on'"); pubnames = options->proto.logical.publication_names; @@ -460,7 +461,7 @@ libpqrcv_startstreaming(WalReceiverConn *conn, pfree(pubnames_str); if (options->proto.logical.binary && - PQserverVersion(conn->streamConn) >= 140000) + server_version >= 140000) appendStringInfoString(&cmd, ", binary 'true'"); appendStringInfoChar(&cmd, ')'); diff --git a/src/bin/pg_basebackup/pg_basebackup.c b/src/bin/pg_basebackup/pg_basebackup.c index 8bb0acf..e08fcb9 100644 --- a/src/bin/pg_basebackup/pg_basebackup.c +++ b/src/bin/pg_basebackup/pg_basebackup.c @@ -597,6 +597,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) uint32 hi, lo; char statusdir[MAXPGPATH]; + int server_version; param = pg_malloc0(sizeof(logstreamer_param)); param->timeline = timeline; @@ -628,14 +629,16 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) /* Error message already written in GetConnection() */ exit(1); + server_version = PQserverVersion(conn); + /* In post-10 cluster, pg_xlog has been renamed to pg_wal */ snprintf(param->xlog, sizeof(param->xlog), "%s/%s", basedir, - PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ? + server_version < MINIMUM_VERSION_FOR_PG_WAL ? "pg_xlog" : "pg_wal"); /* Temporary replication slots are only supported in 10 and newer */ - if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_TEMP_SLOTS) + if (server_version < MINIMUM_VERSION_FOR_TEMP_SLOTS) temp_replication_slot = false; /* @@ -670,7 +673,7 @@ StartLogStreamer(char *startpos, uint32 timeline, char *sysidentifier) */ snprintf(statusdir, sizeof(statusdir), "%s/%s/archive_status", basedir, - PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ? + server_version < MINIMUM_VERSION_FOR_PG_WAL ? "pg_xlog" : "pg_wal"); if (pg_mkdir_p(statusdir, pg_dir_create_mode) != 0 && errno != EEXIST) @@ -2262,6 +2265,7 @@ main(int argc, char **argv) int c; int option_index; + int server_version; pg_logging_init(argv[0]); progname = get_progname(argv[0]); @@ -2595,6 +2599,7 @@ main(int argc, char **argv) exit(1); } atexit(disconnect_atexit); + server_version = PQserverVersion(conn); /* * Set umask so that directories/files are created with the same @@ -2607,7 +2612,7 @@ main(int argc, char **argv) umask(pg_mode_mask); /* Backup manifests are supported in 13 and newer versions */ - if (PQserverVersion(conn) < MINIMUM_VERSION_FOR_MANIFESTS) + if (server_version < MINIMUM_VERSION_FOR_MANIFESTS) manifest = false; /* @@ -2634,7 +2639,7 @@ main(int argc, char **argv) * renamed to pg_wal in post-10 clusters. */ linkloc = psprintf("%s/%s", basedir, - PQserverVersion(conn) < MINIMUM_VERSION_FOR_PG_WAL ? + server_version < MINIMUM_VERSION_FOR_PG_WAL ? "pg_xlog" : "pg_wal"); #ifdef HAVE_SYMLINK diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index fc06815..52f9b48 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -344,10 +344,12 @@ reindex_one_database(ConnParams *cparams, ReindexType type, ParallelSlotArray *sa; bool failed = false; int items_count = 0; + int server_version; conn = connectDatabase(cparams, progname, echo, false, false); + server_version = PQserverVersion(conn); - if (concurrently && PQserverVersion(conn) < 120000) + if (concurrently && server_version < 120000) { PQfinish(conn); pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", @@ -355,7 +357,7 @@ reindex_one_database(ConnParams *cparams, ReindexType type, exit(1); } - if (tablespace && PQserverVersion(conn) < 140000) + if (tablespace && server_version < 140000) { PQfinish(conn); pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c index 61974ba..df5d4c7 100644 --- a/src/bin/scripts/vacuumdb.c +++ b/src/bin/scripts/vacuumdb.c @@ -448,6 +448,7 @@ vacuum_one_database(ConnParams *cparams, bool failed = false; bool tables_listed = false; bool has_where = false; + int server_version; const char *initcmd; const char *stage_commands[] = { "SET default_statistics_target=1; SET vacuum_cost_delay=0;", @@ -464,8 +465,9 @@ vacuum_one_database(ConnParams *cparams, (stage >= 0 && stage < ANALYZE_NUM_STAGES)); conn = connectDatabase(cparams, progname, echo, false, true); + server_version = PQserverVersion(conn); - if (vacopts->disable_page_skipping && PQserverVersion(conn) < 90600) + if (vacopts->disable_page_skipping && server_version < 90600) { PQfinish(conn); pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", @@ -473,7 +475,7 @@ vacuum_one_database(ConnParams *cparams, exit(1); } - if (vacopts->no_index_cleanup && PQserverVersion(conn) < 120000) + if (vacopts->no_index_cleanup && server_version < 120000) { PQfinish(conn); pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", @@ -481,7 +483,7 @@ vacuum_one_database(ConnParams *cparams, exit(1); } - if (vacopts->force_index_cleanup && PQserverVersion(conn) < 120000) + if (vacopts->force_index_cleanup && server_version < 120000) { PQfinish(conn); pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", @@ -489,7 +491,7 @@ vacuum_one_database(ConnParams *cparams, exit(1); } - if (!vacopts->do_truncate && PQserverVersion(conn) < 120000) + if (!vacopts->do_truncate && server_version < 120000) { PQfinish(conn); pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", @@ -497,7 +499,7 @@ vacuum_one_database(ConnParams *cparams, exit(1); } - if (!vacopts->process_toast && PQserverVersion(conn) < 140000) + if (!vacopts->process_toast && server_version < 140000) { PQfinish(conn); pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", @@ -505,7 +507,7 @@ vacuum_one_database(ConnParams *cparams, exit(1); } - if (vacopts->skip_locked && PQserverVersion(conn) < 120000) + if (vacopts->skip_locked && server_version < 120000) { PQfinish(conn); pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", @@ -513,21 +515,21 @@ vacuum_one_database(ConnParams *cparams, exit(1); } - if (vacopts->min_xid_age != 0 && PQserverVersion(conn) < 90600) + if (vacopts->min_xid_age != 0 && server_version < 90600) { pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", "--min-xid-age", "9.6"); exit(1); } - if (vacopts->min_mxid_age != 0 && PQserverVersion(conn) < 90600) + if (vacopts->min_mxid_age != 0 && server_version < 90600) { pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", "--min-mxid-age", "9.6"); exit(1); } - if (vacopts->parallel_workers >= 0 && PQserverVersion(conn) < 130000) + if (vacopts->parallel_workers >= 0 && server_version < 130000) { pg_log_error("cannot use the \"%s\" option on server versions older than PostgreSQL %s", "--parallel", "13"); diff --git a/src/fe_utils/string_utils.c b/src/fe_utils/string_utils.c index 3efee4e..222845d 100644 --- a/src/fe_utils/string_utils.c +++ b/src/fe_utils/string_utils.c @@ -832,6 +832,7 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern, PQExpBufferData schemabuf; PQExpBufferData namebuf; bool added_clause = false; + int server_version = PQserverVersion(conn); #define WHEREAND() \ (appendPQExpBufferStr(buf, have_where ? " AND " : "WHERE "), \ @@ -883,13 +884,13 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern, appendPQExpBuffer(buf, "(%s OPERATOR(pg_catalog.~) ", namevar); appendStringLiteralConn(buf, namebuf.data, conn); - if (PQserverVersion(conn) >= 120000) + if (server_version >= 120000) appendPQExpBufferStr(buf, " COLLATE pg_catalog.default"); appendPQExpBuffer(buf, "\n OR %s OPERATOR(pg_catalog.~) ", altnamevar); appendStringLiteralConn(buf, namebuf.data, conn); - if (PQserverVersion(conn) >= 120000) + if (server_version >= 120000) appendPQExpBufferStr(buf, " COLLATE pg_catalog.default"); appendPQExpBufferStr(buf, ")\n"); } @@ -897,7 +898,7 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern, { appendPQExpBuffer(buf, "%s OPERATOR(pg_catalog.~) ", namevar); appendStringLiteralConn(buf, namebuf.data, conn); - if (PQserverVersion(conn) >= 120000) + if (server_version >= 120000) appendPQExpBufferStr(buf, " COLLATE pg_catalog.default"); appendPQExpBufferChar(buf, '\n'); } @@ -914,7 +915,7 @@ processSQLNamePattern(PGconn *conn, PQExpBuffer buf, const char *pattern, WHEREAND(); appendPQExpBuffer(buf, "%s OPERATOR(pg_catalog.~) ", schemavar); appendStringLiteralConn(buf, schemabuf.data, conn); - if (PQserverVersion(conn) >= 120000) + if (server_version >= 120000) appendPQExpBufferStr(buf, " COLLATE pg_catalog.default"); appendPQExpBufferChar(buf, '\n'); } -- 1.8.3.1