More consistent error reporting

schmiddy · dvarrazzo · commit 3c73a0204af0 · 2012-11-16T15:37:31.000Z
This patch is a port of Daniele's commit 0be414ad10c32d from his own fork,
"error_reporting" branch.

reorg_all_database can return an error message: in case of any error different
from "missing schema" return the error and keep processing the other databases
instead of printing and stopping the program.

The output of the program is now something like:

    $ pg_reorg --all
    pg_reorg: reorg database "contrib_regression"
    pg_reorg: reorg database "template1" ... skipped: pg_reorg is not installed in the database
diff --git a/bin/pg_repack.c b/bin/pg_repack.c
@@ -107,7 +107,7 @@ typedef struct repack_index
 } repack_index;
 
 static void repack_all_databases(const char *order_by);
-static bool repack_one_database(const char *order_by);
+static bool repack_one_database(const char *order_by, char *errbuf, size_t errsize);
 static void repack_one_table(const repack_table *table, const char *order_by);
 static void repack_cleanup(bool fatal, const repack_table *table);
 
@@ -178,10 +178,11 @@ main(int argc, char *argv[])
 	}
 	else
 	{
-		if (!repack_one_database(orderby))
+		char errbuf[256];
+		if (!repack_one_database(orderby, errbuf, sizeof(errbuf)))
 			ereport(ERROR,
-					(errcode(ENOENT),
-					 errmsg("%s is not installed", PROGRAM_NAME)));
+					(errcode(ERROR),
+					 errmsg("%s", errbuf)));
 	}
 
 	return 0;
@@ -204,23 +205,24 @@ repack_all_databases(const char *orderby)
 	for (i = 0; i < PQntuples(result); i++)
 	{
 		bool	ret;
+		char	errbuf[256];
 
 		dbname = PQgetvalue(result, i, 0);
 
 		if (pgut_log_level >= INFO)
 		{
-			printf("%s: repack database \"%s\"", PROGRAM_NAME, dbname);
+			printf("%s: repack database \"%s\"\n", PROGRAM_NAME, dbname);
 			fflush(stdout);
 		}
 
-		ret = repack_one_database(orderby);
+		ret = repack_one_database(orderby, errbuf, sizeof(errbuf));
 
 		if (pgut_log_level >= INFO)
 		{
 			if (ret)
 				printf("\n");
 			else
-				printf(" ... skipped\n");
+				printf(" ... skipped: %s\n", errbuf);
 			fflush(stdout);
 		}
 	}
@@ -252,7 +254,7 @@ getoid(PGresult *res, int row, int col)
  * Call repack_one_table for the target table or each table in a database.
  */
 static bool
-repack_one_database(const char *orderby)
+repack_one_database(const char *orderby, char *errbuf, size_t errsize)
 {
 	bool					ret = true;
 	PGresult	   		   *res;
@@ -303,22 +305,25 @@ repack_one_database(const char *orderby)
 		res = execute_elevel(sql.data, 0, NULL, DEBUG2);
 	}
 
+	/* on error skip the database */
 	if (PQresultStatus(res) != PGRES_TUPLES_OK)
 	{
 		if (sqlstate_equals(res, SQLSTATE_INVALID_SCHEMA_NAME))
 		{
 			/* Schema repack does not exist. Skip the database. */
-			PQclear(res);
-			ret = false;
-			goto cleanup;
+			if (errbuf)
+				snprintf(errbuf, errsize,
+						 "%s is not installed in the database", PROGRAM_NAME);
+
 		}
 		else
 		{
-			/* exit otherwise */
-			elog(ERROR, "%s", PQerrorMessage(connection));
-			PQclear(res);
-			exit(1);
+			/* Return the error message otherwise */
+			if (errbuf)
+				snprintf(errbuf, errsize, "%s", PQerrorMessage(connection));
 		}
+		ret = false;
+		goto cleanup;
 	}
 
 	num = PQntuples(res);

Original file line number	Diff line number	Diff line change
`@@ -107,7 +107,7 @@ typedef struct repack_index`
`107`	`107`	`} repack_index;`
`108`	`108`
`109`	`109`	`static void repack_all_databases(const char *order_by);`
`110`		`-static bool repack_one_database(const char *order_by);`
	`110`	`+static bool repack_one_database(const char order_by, char errbuf, size_t errsize);`
`111`	`111`	`static void repack_one_table(const repack_table table, const char order_by);`
`112`	`112`	`static void repack_cleanup(bool fatal, const repack_table *table);`
`113`	`113`
`@@ -178,10 +178,11 @@ main(int argc, char *argv[])`
`178`	`178`	`}`
`179`	`179`	`else`
`180`	`180`	`{`
`181`		`- if (!repack_one_database(orderby))`
	`181`	`+ char errbuf[256];`
	`182`	`+ if (!repack_one_database(orderby, errbuf, sizeof(errbuf)))`
`182`	`183`	`ereport(ERROR,`
`183`		`- (errcode(ENOENT),`
`184`		`- errmsg("%s is not installed", PROGRAM_NAME)));`
	`184`	`+ (errcode(ERROR),`
	`185`	`+ errmsg("%s", errbuf)));`
`185`	`186`	`}`
`186`	`187`
`187`	`188`	`return 0;`
`@@ -204,23 +205,24 @@ repack_all_databases(const char *orderby)`
`204`	`205`	`for (i = 0; i < PQntuples(result); i++)`
`205`	`206`	`{`
`206`	`207`	`bool ret;`
	`208`	`+ char errbuf[256];`
`207`	`209`
`208`	`210`	`dbname = PQgetvalue(result, i, 0);`
`209`	`211`
`210`	`212`	`if (pgut_log_level >= INFO)`
`211`	`213`	`{`
`212`		`- printf("%s: repack database \"%s\"", PROGRAM_NAME, dbname);`
	`214`	`+ printf("%s: repack database \"%s\"\n", PROGRAM_NAME, dbname);`
`213`	`215`	`fflush(stdout);`
`214`	`216`	`}`
`215`	`217`
`216`		`- ret = repack_one_database(orderby);`
	`218`	`+ ret = repack_one_database(orderby, errbuf, sizeof(errbuf));`
`217`	`219`
`218`	`220`	`if (pgut_log_level >= INFO)`
`219`	`221`	`{`
`220`	`222`	`if (ret)`
`221`	`223`	`printf("\n");`
`222`	`224`	`else`
`223`		`- printf(" ... skipped\n");`
	`225`	`+ printf(" ... skipped: %s\n", errbuf);`
`224`	`226`	`fflush(stdout);`
`225`	`227`	`}`
`226`	`228`	`}`
`@@ -252,7 +254,7 @@ getoid(PGresult *res, int row, int col)`
`252`	`254`	`* Call repack_one_table for the target table or each table in a database.`
`253`	`255`	`*/`
`254`	`256`	`static bool`
`255`		`-repack_one_database(const char *orderby)`
	`257`	`+repack_one_database(const char orderby, char errbuf, size_t errsize)`
`256`	`258`	`{`
`257`	`259`	`bool ret = true;`
`258`	`260`	`PGresult *res;`
`@@ -303,22 +305,25 @@ repack_one_database(const char *orderby)`
`303`	`305`	`res = execute_elevel(sql.data, 0, NULL, DEBUG2);`
`304`	`306`	`}`
`305`	`307`
	`308`	`+ /* on error skip the database */`
`306`	`309`	`if (PQresultStatus(res) != PGRES_TUPLES_OK)`
`307`	`310`	`{`
`308`	`311`	`if (sqlstate_equals(res, SQLSTATE_INVALID_SCHEMA_NAME))`
`309`	`312`	`{`
`310`	`313`	`/* Schema repack does not exist. Skip the database. */`
`311`		`- PQclear(res);`
`312`		`- ret = false;`
`313`		`- goto cleanup;`
	`314`	`+ if (errbuf)`
	`315`	`+ snprintf(errbuf, errsize,`
	`316`	`+ "%s is not installed in the database", PROGRAM_NAME);`
	`317`	`+`
`314`	`318`	`}`
`315`	`319`	`else`
`316`	`320`	`{`
`317`		`- /* exit otherwise */`
`318`		`- elog(ERROR, "%s", PQerrorMessage(connection));`
`319`		`- PQclear(res);`
`320`		`- exit(1);`
	`321`	`+ /* Return the error message otherwise */`
	`322`	`+ if (errbuf)`
	`323`	`+ snprintf(errbuf, errsize, "%s", PQerrorMessage(connection));`
`321`	`324`	`}`
	`325`	`+ ret = false;`
	`326`	`+ goto cleanup;`
`322`	`327`	`}`
`323`	`328`
`324`	`329`	`num = PQntuples(res);`