From 9db314397302bc01d81a24cb0edaceefa2b2e9d0 Mon Sep 17 00:00:00 2001 From: David Steele Date: Thu, 17 Sep 2020 10:35:27 -0400 Subject: [PATCH] Error with hints when backup user cannot read pg_settings. This condition used to give a not-very-clear error which we have been intending to improve. But in the meantime the changes in fbff299 resulted in a segfault for this condition instead because the data_directory was assumed to be non-NULL. Fix this by explicitly throwing an error with hints when any row in pg_settings cannot be selected. --- doc/xml/release.xml | 18 ++++++++++++++ src/db/db.c | 13 ++++++++++ test/src/module/db/dbTest.c | 48 +++++++++++++++++++++++++++++++++++++ 3 files changed, 79 insertions(+) diff --git a/doc/xml/release.xml b/doc/xml/release.xml index a83b03c14..c89201f77 100644 --- a/doc/xml/release.xml +++ b/doc/xml/release.xml @@ -14,6 +14,19 @@ + + + + + + + + + +

Error with hints when backup user cannot read pg_settings.

+ + + @@ -9044,6 +9057,11 @@ Yuxael + + Mohamed Insaf K + insafk + + Mohamad El-Rifai melrifa1 diff --git a/src/db/db.c b/src/db/db.c index 473c0473e..de3037d36 100644 --- a/src/db/db.c +++ b/src/db/db.c @@ -219,6 +219,19 @@ dbOpen(Db *this) " (select setting from pg_catalog.pg_settings where name = 'archive_mode')::text," " (select setting from pg_catalog.pg_settings where name = 'archive_command')::text")); + // Check that none of the return values are null, which indicates the user cannot select some rows in pg_settings + for (unsigned int columnIdx = 0; columnIdx < varLstSize(row); columnIdx++) + { + if (varLstGet(row, columnIdx) == NULL) + { + THROW( + DbQueryError, + "unable to select some rows from pg_settings\n" + "HINT: is the backup running as the postgres user?\n" + "HINT: is the pg_read_all_settings role assigned for " PG_NAME " >= " PG_VERSION_10_STR "?"); + } + } + // Strip the minor version off since we don't need it. In the future it might be a good idea to warn users when they are // running an old minor version. this->pgVersion = varUIntForce(varLstGet(row, 0)) / 100 * 100; diff --git a/test/src/module/db/dbTest.c b/test/src/module/db/dbTest.c index 5cc723622..28ca3070e 100644 --- a/test/src/module/db/dbTest.c +++ b/test/src/module/db/dbTest.c @@ -145,6 +145,54 @@ testRun(void) strLstAddZ(argList, "--pg1-path=/pg1"); harnessCfgLoad(cfgCmdBackup, argList); + // ------------------------------------------------------------------------------------------------------------------------- + TEST_TITLE("error when unable to select any pg_settings"); + + harnessPqScriptSet((HarnessPq []) + { + // Connect to primary + HRNPQ_MACRO_OPEN(1, "dbname='postgres' port=5432"), + HRNPQ_MACRO_SET_SEARCH_PATH(1), + HRNPQ_MACRO_SET_CLIENT_ENCODING(1), + + // Return NULL for a row in pg_settings + {.session = 1, .function = HRNPQ_SENDQUERY, .param = + "[\"select (select setting from pg_catalog.pg_settings where name = 'server_version_num')::int4," + " (select setting from pg_catalog.pg_settings where name = 'data_directory')::text," + " (select setting from pg_catalog.pg_settings where name = 'archive_mode')::text," + " (select setting from pg_catalog.pg_settings where name = 'archive_command')::text\"]", + .resultInt = 1}, + {.session = 1, .function = HRNPQ_CONSUMEINPUT}, + {.session = 1, .function = HRNPQ_ISBUSY}, + {.session = 1, .function = HRNPQ_GETRESULT}, + {.session = 1, .function = HRNPQ_RESULTSTATUS, .resultInt = PGRES_TUPLES_OK}, + {.session = 1, .function = HRNPQ_NTUPLES, .resultInt = 1}, + {.session = 1, .function = HRNPQ_NFIELDS, .resultInt = 4}, + {.session = 1, .function = HRNPQ_FTYPE, .param = "[0]", .resultInt = HRNPQ_TYPE_INT}, + {.session = 1, .function = HRNPQ_FTYPE, .param = "[1]", .resultInt = HRNPQ_TYPE_TEXT}, + {.session = 1, .function = HRNPQ_FTYPE, .param = "[2]", .resultInt = HRNPQ_TYPE_TEXT}, + {.session = 1, .function = HRNPQ_FTYPE, .param = "[3]", .resultInt = HRNPQ_TYPE_TEXT}, + {.session = 1, .function = HRNPQ_GETVALUE, .param = "[0,0]", .resultZ = "0"}, + {.session = 1, .function = HRNPQ_GETVALUE, .param = "[0,1]", .resultZ = "value"}, + {.session = 1, .function = HRNPQ_GETVALUE, .param = "[0,2]", .resultZ = "value"}, + {.session = 1, .function = HRNPQ_GETVALUE, .param = "[0,3]", .resultZ = ""}, + {.session = 1, .function = HRNPQ_GETISNULL, .param = "[0,3]", .resultInt = 1}, + {.session = 1, .function = HRNPQ_CLEAR}, + {.session = 1, .function = HRNPQ_GETRESULT, .resultNull = true}, + + // Close primary + HRNPQ_MACRO_CLOSE(1), + + HRNPQ_MACRO_DONE() + }); + + TEST_ERROR(dbGet(true, true, false), DbConnectError, "unable to find primary cluster - cannot proceed"); + + TEST_RESULT_LOG( + "P00 WARN: unable to check pg-1: [DbQueryError] unable to select some rows from pg_settings\n" + " HINT: is the backup running as the postgres user?\n" + " HINT: is the pg_read_all_settings role assigned for PostgreSQL >= 10?"); + // ------------------------------------------------------------------------------------------------------------------------- TEST_TITLE("PostgreSQL 8.3 start backup with no start fast");