From c43b6bdceb93d4d3bf0322819ccf41e8f720f439 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 18 Oct 2012 00:18:27 +0100 Subject: [PATCH 1/3] More consistent error reporting repack_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 --- bin/pg_repack.c | 31 ++++++++++++++++++------------- doc/pg_repack.rst | 2 +- 2 files changed, 19 insertions(+), 14 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 5960ab9..98ce7b5 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -83,7 +83,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, const char *table); +static bool repack_one_database(const char *order_by, const char *table, char *errbuf, size_t errsize); static void repack_one_table(const repack_table *table, const char *order_by); static void repack_cleanup(bool fatal, void *userdata); @@ -152,10 +152,11 @@ main(int argc, char *argv[]) } else { - if (!repack_one_database(orderby, table)) + char errbuf[256]; + if (!repack_one_database(orderby, table, errbuf, sizeof(errbuf))) ereport(ERROR, - (errcode(ENOENT), - errmsg("%s is not installed", PROGRAM_NAME))); + (errcode(ERROR), + errmsg("%s", errbuf))); } return 0; @@ -178,6 +179,7 @@ repack_all_databases(const char *orderby) for (i = 0; i < PQntuples(result); i++) { bool ret; + char errbuf[256]; dbname = PQgetvalue(result, i, 0); @@ -187,14 +189,14 @@ repack_all_databases(const char *orderby) fflush(stdout); } - ret = repack_one_database(orderby, NULL); + ret = repack_one_database(orderby, NULL, errbuf, sizeof(errbuf)); if (pgut_log_level >= INFO) { if (ret) printf("\n"); else - printf(" ... skipped\n"); + printf(" ... skipped: %s\n", errbuf); fflush(stdout); } } @@ -225,7 +227,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, const char *table) +repack_one_database(const char *orderby, const char *table, char *errbuf, size_t errsize) { bool ret = true; PGresult *res; @@ -261,21 +263,24 @@ repack_one_database(const char *orderby, const char *table) 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. */ - ret = false; - goto cleanup; + if (errbuf) + snprintf(errbuf, errsize, + "%s is not installed in the database", PROGRAM_NAME); } else { - /* exit otherwise */ - printf("%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); diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 672b89d..7820593 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -270,7 +270,7 @@ version load the script ``$SHAREDIR/contrib/uninstall_pg_repack.sql`` into the database where the error occured and then load ``$SHAREDIR/contrib/pg_repack.sql`` again. -pg_repack: repack database "template1" ... skipped +pg_repack: reorg database "template1" ... skipped: pg_repack is not installed in the database pg_repack is not installed in the database when ``--all`` option is specified. From 1bcaf267b389a0e05fdfce2fca5c0a192a8cbb51 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 18 Oct 2012 00:43:45 +0100 Subject: [PATCH 2/3] Stop database processing if library version doesn't match the binary Actually this leaves out the case of the SQL schema not consistent with the library/binary installed, and this is a relatively likely case: the user has run "make install" but the repack schema was already loaded from an older version. --- bin/pg_repack.c | 40 +++++++++++++++++++++++++++++++++++++++- doc/pg_repack.rst | 9 +++++++++ 2 files changed, 48 insertions(+), 1 deletion(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 98ce7b5..afcb030 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -229,7 +229,7 @@ getoid(PGresult *res, int row, int col) static bool repack_one_database(const char *orderby, const char *table, char *errbuf, size_t errsize) { - bool ret = true; + bool ret = false; PGresult *res; int i; int num; @@ -239,6 +239,43 @@ repack_one_database(const char *orderby, const char *table, char *errbuf, size_t reconnect(ERROR); + /* Query the extension version. Exit if no match */ + res = execute_elevel("select repack.version()", 0, NULL, DEBUG2); + if (PQresultStatus(res) == PGRES_TUPLES_OK) + { + const char *libver; + char buf[64]; + + /* the string is something like "pg_repack 1.1.7" */ + libver = getstr(res, 0, 0); + snprintf(buf, sizeof(buf), "%s %s", PROGRAM_NAME, PROGRAM_VERSION); + if (0 != strcmp(buf, libver)) + { + if (errbuf) + snprintf(errbuf, errsize, + "program '%s' does not match database library '%s'", + buf, libver); + goto cleanup; + } + } + else + { + if (sqlstate_equals(res, SQLSTATE_INVALID_SCHEMA_NAME)) + { + /* Schema repack does not exist. Skip the database. */ + if (errbuf) + snprintf(errbuf, errsize, + "%s is not installed in the database", PROGRAM_NAME); + } + else + { + /* Return the error message otherwise */ + if (errbuf) + snprintf(errbuf, errsize, "%s", PQerrorMessage(connection)); + } + goto cleanup; + } + /* Disable statement timeout. */ command("SET statement_timeout = 0", 0, NULL); @@ -346,6 +383,7 @@ repack_one_database(const char *orderby, const char *table, char *errbuf, size_t repack_one_table(&table, orderby); } + ret = true; cleanup: PQclear(res); diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 7820593..6682f3c 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -281,6 +281,15 @@ ERROR: pg_repack is not installed Do register pg_repack to the database. +ERROR: program 'pg_repack V1' does not match database library 'pg_repack V2' + There is a mismatch between the ``pg_repack`` binary and the database + library (``.so`` or ``.dll``). + + The mismatch could be due to the wrong binary in the ``$PATH`` or the + wrong database being addressed. Check the program directory and the + database; if they are what expected you may need to repeat pg_repack + installation. + ERROR: relation "table" has no primary key The target table doesn't have PRIMARY KEY. From deaae7dd7224fe83df80067c9cdcae0e3583844e Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 15 Nov 2012 23:37:09 +0000 Subject: [PATCH 3/3] Added version_sql() function and consistency check of sql version --- bin/pg_repack.c | 18 ++++++++++++++++-- doc/pg_repack.rst | 7 +++++++ lib/Makefile | 7 ++++--- lib/pg_repack.sql.in | 4 ++++ 4 files changed, 31 insertions(+), 5 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index afcb030..98d3aa5 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -240,15 +240,18 @@ repack_one_database(const char *orderby, const char *table, char *errbuf, size_t reconnect(ERROR); /* Query the extension version. Exit if no match */ - res = execute_elevel("select repack.version()", 0, NULL, DEBUG2); + res = execute_elevel("select repack.version(), repack.version_sql()", + 0, NULL, DEBUG2); if (PQresultStatus(res) == PGRES_TUPLES_OK) { const char *libver; char buf[64]; /* the string is something like "pg_repack 1.1.7" */ - libver = getstr(res, 0, 0); snprintf(buf, sizeof(buf), "%s %s", PROGRAM_NAME, PROGRAM_VERSION); + + /* check the version of the C library */ + libver = getstr(res, 0, 0); if (0 != strcmp(buf, libver)) { if (errbuf) @@ -257,6 +260,17 @@ repack_one_database(const char *orderby, const char *table, char *errbuf, size_t buf, libver); goto cleanup; } + + /* check the version of the SQL extension */ + libver = getstr(res, 0, 1); + if (0 != strcmp(buf, libver)) + { + if (errbuf) + snprintf(errbuf, errsize, + "extension '%s' required, found extension '%s'", + buf, libver); + goto cleanup; + } } else { diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 6682f3c..3adb41e 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -290,6 +290,13 @@ ERROR: program 'pg_repack V1' does not match database library 'pg_repack V2' database; if they are what expected you may need to repeat pg_repack installation. +ERROR: extension 'pg_repack V1' required, found extension 'pg_repack V2' + The SQL extension found in the database does not match the version + required by the pg_repack program. + + You should drop the extension from the database and reload it as described + in the installation_ section. + ERROR: relation "table" has no primary key The target table doesn't have PRIMARY KEY. diff --git a/lib/Makefile b/lib/Makefile index 045cc0a..7b9a92b 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -30,7 +30,7 @@ DATA_built = pg_repack.sql DATA = uninstall_pg_repack.sql endif -USE_PGXS = 1 # use pgxs if not in contrib directory +USE_PGXS = 1 PGXS := $(shell $(PG_CONFIG) --pgxs) include $(PGXS) @@ -40,11 +40,12 @@ LIBS := $(filter-out -lxslt, $(LIBS)) pg_repack.sql: pg_repack.sql.in echo "BEGIN;\n" > $@; \ - sed 's,MODULE_PATHNAME,$$libdir/$(MODULE_big),g' $< >> $@; \ + sed 's,MODULE_PATHNAME,$$libdir/$(MODULE_big),g' $< \ + | sed 's,REPACK_VERSION,$(REPACK_VERSION),g' >> $@; \ echo "\nCOMMIT;" >> $@; pg_repack--$(REPACK_VERSION).sql: pg_repack.sql.in - cat $< > $@; + sed 's,REPACK_VERSION,$(REPACK_VERSION),g' $< > $@; pg_repack.control: pg_repack.control.in sed 's,REPACK_VERSION,$(REPACK_VERSION),g' $< > $@ diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index 52514f2..3f78d19 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -12,6 +12,10 @@ CREATE FUNCTION repack.version() RETURNS text AS 'MODULE_PATHNAME', 'repack_version' LANGUAGE C IMMUTABLE STRICT; +CREATE FUNCTION repack.version_sql() RETURNS text AS +$$SELECT 'pg_repack REPACK_VERSION'::text$$ +LANGUAGE SQL IMMUTABLE STRICT; + CREATE AGGREGATE repack.array_accum ( sfunc = array_append, basetype = anyelement,