Refactor check_ functions to use filehandle for status

When reporting failure in check_ functions there is (typically) a text-
file mentioned in the error report which contains further details. Some
check_ functions kept a separate flag variable to indicate failure, and
some just checked the state of the filehandle as it's guaranteed to be
open when the check failed. This refactors the functions to consistently
do the same check on error reporting. As the error report contains the
filepath, it makes more sense to check the filehandle state and skip the
flag variable.

Reviewed-by: Nathan Bossart <[email protected]>
Reviewed-by: Bruce Momjian <[email protected]>
Discussion: https://postgr.es/m/595759F6-625B-4ED7-8125-91AF00437F83@yesql.se

diff --git a/src/bin/pg_upgrade/check.c b/src/bin/pg_upgrade/check.c
index 0ed0590d82dae1c051cd4ec9c36debc8d4b7041e..f4969bcdad7a34e38e117aa62484690f7e14d9eb 100644 (file)
--- a/src/bin/pg_upgrade/check.c
+++ b/src/bin/pg_upgrade/check.c
@@ -711,7 +711,6 @@ check_proper_datallowconn(ClusterInfo *cluster)
        int                     i_datallowconn;
        FILE       *script = NULL;
        char            output_path[MAXPGPATH];
-       bool            found = false;
 
        prep_status("Checking database connection settings");
 
@@ -750,7 +749,6 @@ check_proper_datallowconn(ClusterInfo *cluster)
                         */
                        if (strcmp(datallowconn, "f") == 0)
                        {
-                               found = true;
                                if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
                                        pg_fatal("could not open file \"%s\": %s",
                                                         output_path, strerror(errno));
@@ -765,10 +763,8 @@ check_proper_datallowconn(ClusterInfo *cluster)
        PQfinish(conn_template1);
 
        if (script)
-               fclose(script);
-
-       if (found)
        {
+               fclose(script);
                pg_log(PG_REPORT, "fatal");
                pg_fatal("All non-template0 databases must allow connections, i.e. their\n"
                                 "pg_database.datallowconn must be true.  Your installation contains\n"
@@ -829,7 +825,6 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster)
 {
        int                     dbnum;
        FILE       *script = NULL;
-       bool            found = false;
        char            output_path[MAXPGPATH];
 
        prep_status("Checking for contrib/isn with bigint-passing mismatch");
@@ -870,7 +865,6 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster)
                i_proname = PQfnumber(res, "proname");
                for (rowno = 0; rowno < ntups; rowno++)
                {
-                       found = true;
                        if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
                                pg_fatal("could not open file \"%s\": %s",
                                                 output_path, strerror(errno));
@@ -890,10 +884,8 @@ check_for_isn_and_int8_passing_mismatch(ClusterInfo *cluster)
        }
 
        if (script)
-               fclose(script);
-
-       if (found)
        {
+               fclose(script);
                pg_log(PG_REPORT, "fatal");
                pg_fatal("Your installation contains \"contrib/isn\" functions which rely on the\n"
                                 "bigint data type.  Your old and new clusters pass bigint values\n"
@@ -915,7 +907,6 @@ check_for_user_defined_postfix_ops(ClusterInfo *cluster)
 {
        int                     dbnum;
        FILE       *script = NULL;
-       bool            found = false;
        char            output_path[MAXPGPATH];
 
        prep_status("Checking for user-defined postfix operators");
@@ -968,7 +959,6 @@ check_for_user_defined_postfix_ops(ClusterInfo *cluster)
                i_typname = PQfnumber(res, "typname");
                for (rowno = 0; rowno < ntups; rowno++)
                {
-                       found = true;
                        if (script == NULL &&
                                (script = fopen_priv(output_path, "w")) == NULL)
                                pg_fatal("could not open file \"%s\": %s",
@@ -992,10 +982,8 @@ check_for_user_defined_postfix_ops(ClusterInfo *cluster)
        }
 
        if (script)
-               fclose(script);
-
-       if (found)
        {
+               fclose(script);
                pg_log(PG_REPORT, "fatal");
                pg_fatal("Your installation contains user-defined postfix operators, which are not\n"
                                 "supported anymore.  Consider dropping the postfix operators and replacing\n"
@@ -1145,7 +1133,6 @@ check_for_tables_with_oids(ClusterInfo *cluster)
 {
        int                     dbnum;
        FILE       *script = NULL;
-       bool            found = false;
        char            output_path[MAXPGPATH];
 
        prep_status("Checking for tables WITH OIDS");
@@ -1179,7 +1166,6 @@ check_for_tables_with_oids(ClusterInfo *cluster)
                i_relname = PQfnumber(res, "relname");
                for (rowno = 0; rowno < ntups; rowno++)
                {
-                       found = true;
                        if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
                                pg_fatal("could not open file \"%s\": %s",
                                                 output_path, strerror(errno));
@@ -1199,10 +1185,8 @@ check_for_tables_with_oids(ClusterInfo *cluster)
        }
 
        if (script)
-               fclose(script);
-
-       if (found)
        {
+               fclose(script);
                pg_log(PG_REPORT, "fatal");
                pg_fatal("Your installation contains tables declared WITH OIDS, which is not\n"
                                 "supported anymore.  Consider removing the oid column using\n"
@@ -1401,7 +1385,6 @@ check_for_user_defined_encoding_conversions(ClusterInfo *cluster)
 {
        int                     dbnum;
        FILE       *script = NULL;
-       bool            found = false;
        char            output_path[MAXPGPATH];
 
        prep_status("Checking for user-defined encoding conversions");
@@ -1441,7 +1424,6 @@ check_for_user_defined_encoding_conversions(ClusterInfo *cluster)
                i_nspname = PQfnumber(res, "nspname");
                for (rowno = 0; rowno < ntups; rowno++)
                {
-                       found = true;
                        if (script == NULL &&
                                (script = fopen_priv(output_path, "w")) == NULL)
                                pg_fatal("could not open file \"%s\": %s",
@@ -1463,10 +1445,8 @@ check_for_user_defined_encoding_conversions(ClusterInfo *cluster)
        }
 
        if (script)
-               fclose(script);
-
-       if (found)
        {
+               fclose(script);
                pg_log(PG_REPORT, "fatal");
                pg_fatal("Your installation contains user-defined encoding conversions.\n"
                                 "The conversion function parameters changed in PostgreSQL version 14\n"

diff --git a/src/bin/pg_upgrade/function.c b/src/bin/pg_upgrade/function.c
index 70b492cc3a834b83209bf9fbc3677d590acd0a3a..93d975864babaec8d8d3f7fb9e36a294e6d4892d 100644 (file)
--- a/src/bin/pg_upgrade/function.c
+++ b/src/bin/pg_upgrade/function.c
@@ -123,7 +123,6 @@ check_loadable_libraries(void)
        int                     libnum;
        int                     was_load_failure = false;
        FILE       *script = NULL;
-       bool            found = false;
        char            output_path[MAXPGPATH];
 
        prep_status("Checking for presence of required libraries");
@@ -158,7 +157,6 @@ check_loadable_libraries(void)
 
                        if (PQresultStatus(res) != PGRES_COMMAND_OK)
                        {
-                               found = true;
                                was_load_failure = true;
 
                                if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
@@ -181,7 +179,7 @@ check_loadable_libraries(void)
 
        PQfinish(conn);
 
-       if (found)
+       if (script)
        {
                fclose(script);
                pg_log(PG_REPORT, "fatal");

diff --git a/src/bin/pg_upgrade/version.c b/src/bin/pg_upgrade/version.c
index d2636ba857f29ea52e9e75881e236d8b8c537bf6..064d23797c54d776ab25ba9cd31c8f9959a71857 100644 (file)
--- a/src/bin/pg_upgrade/version.c
+++ b/src/bin/pg_upgrade/version.c
@@ -391,7 +391,6 @@ report_extension_updates(ClusterInfo *cluster)
 {
        int                     dbnum;
        FILE       *script = NULL;
-       bool            found = false;
        char       *output_path = "update_extensions.sql";
 
        prep_status("Checking for extension updates");
@@ -417,8 +416,6 @@ report_extension_updates(ClusterInfo *cluster)
                i_name = PQfnumber(res, "name");
                for (rowno = 0; rowno < ntups; rowno++)
                {
-                       found = true;
-
                        if (script == NULL && (script = fopen_priv(output_path, "w")) == NULL)
                                pg_fatal("could not open file \"%s\": %s", output_path,
                                                 strerror(errno));
@@ -442,10 +439,8 @@ report_extension_updates(ClusterInfo *cluster)
        }
 
        if (script)
-               fclose(script);
-
-       if (found)
        {
+               fclose(script);
                report_status(PG_REPORT, "notice");
                pg_log(PG_REPORT, "\n"
                           "Your installation contains extensions that should be updated\n"