From 700a1a6c48dfc382e3b55a314eb7b13fe8931cd9 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 23 Nov 2012 00:55:53 +0000 Subject: [PATCH 1/5] More explicit error message if the version functions are not found --- bin/pg_repack.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 759434a..ffd74b6 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -186,6 +186,7 @@ static bool kill_ddl(PGconn *conn, Oid relid, bool terminate); static bool lock_access_share(PGconn *conn, Oid relid, const char *target_name); #define SQLSTATE_INVALID_SCHEMA_NAME "3F000" +#define SQLSTATE_UNDEFINED_FUNCTION "42883" #define SQLSTATE_QUERY_CANCELED "57014" static bool sqlstate_equals(PGresult *res, const char *state) @@ -477,12 +478,16 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) } else { - if (sqlstate_equals(res, SQLSTATE_INVALID_SCHEMA_NAME)) + if (sqlstate_equals(res, SQLSTATE_INVALID_SCHEMA_NAME) + || sqlstate_equals(res, SQLSTATE_UNDEFINED_FUNCTION)) { - /* Schema repack does not exist. Skip the database. */ + /* Schema repack does not exist, or version too old (version + * functions not found). Skip the database. + */ if (errbuf) snprintf(errbuf, errsize, - "%s is not installed in the database", PROGRAM_NAME); + "%s %s is not installed in the database", + PROGRAM_NAME, PROGRAM_VERSION); } else { From 2335a4da828bb30d75443df814110dc700da9502 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 23 Nov 2012 01:39:20 +0000 Subject: [PATCH 2/5] Dropped redundant check for missing schema If the schema is missing we have already stopped trying to check the version numbers above. --- bin/pg_repack.c | 16 +++------------- 1 file changed, 3 insertions(+), 13 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index ffd74b6..f50a4ab 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -552,19 +552,9 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) /* 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. */ - 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)); - } + /* Return the error message otherwise */ + if (errbuf) + snprintf(errbuf, errsize, "%s", PQerrorMessage(connection)); goto cleanup; } From 4a3f42ab70026d2557d28506392790b6c71fcfd4 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 17:46:15 +0100 Subject: [PATCH 3/5] Dropped unneeded error check if pgut_execute fails it bails out. --- bin/pg_repack.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index f50a4ab..d40a953 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -1024,12 +1024,6 @@ repack_one_table(const repack_table *table, const char *orderby) * pg_locks momentarily. */ res = pgut_execute(conn2, "SELECT pg_backend_pid()", 0, NULL); - if (PQresultStatus(res) != PGRES_TUPLES_OK) - { - printf("%s", PQerrorMessage(conn2)); - have_error = true; - goto cleanup; - } buffer[0] = '\0'; strncat(buffer, PQgetvalue(res, 0, 0), sizeof(buffer) - 1); CLEARPGRES(res); From 5773c75b58bab36bfe4f83cecffc2de9ff1faa7b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 17:47:44 +0100 Subject: [PATCH 4/5] Using elog instead of printf to report what database we are on I know the ... is pretty but it messes up with warnings etc. By the way there was a \n so the skipped part wasn't even going on the same line. --- bin/pg_repack.c | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index d40a953..681fa44 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -365,22 +365,10 @@ repack_all_databases(const char *orderby) dbname = PQgetvalue(result, i, 0); - if (pgut_log_level >= INFO) - { - printf("%s: repack database \"%s\"\n", PROGRAM_NAME, dbname); - fflush(stdout); - } - + elog(INFO, "repacking database \"%s\"", dbname); ret = repack_one_database(orderby, errbuf, sizeof(errbuf)); - - if (pgut_log_level >= INFO) - { - if (ret) - printf("\n"); - else - printf(" ... skipped: %s\n", errbuf); - fflush(stdout); - } + if (!ret) + elog(INFO, "database \"%s\" skipped: %s", dbname, errbuf); } CLEARPGRES(result); From 08a8c943e548a66a6aed95915fb471e1b1be4077 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 17:48:05 +0100 Subject: [PATCH 5/5] Logging the table we are working on at info level --- bin/expected/repack.out | 12 ++++++++++++ bin/expected/tablespace.out | 4 ++++ bin/pg_repack.c | 2 ++ 3 files changed, 18 insertions(+) diff --git a/bin/expected/repack.out b/bin/expected/repack.out index 5026659..3beabdb 100644 --- a/bin/expected/repack.out +++ b/bin/expected/repack.out @@ -115,9 +115,14 @@ SELECT * FROM tbl_with_dropped_toast; -- do repack -- \! pg_repack --dbname=contrib_regression --table=tbl_badindex +INFO: repacking table "tbl_badindex" WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON tbl_badindex USING btree (n) \! pg_repack --dbname=contrib_regression +INFO: repacking table "tbl_cluster" +INFO: repacking table "tbl_with_dropped_column" +INFO: repacking table "tbl_with_dropped_toast" \! pg_repack --dbname=contrib_regression --table=tbl_cluster +INFO: repacking table "tbl_cluster" -- -- after -- @@ -308,8 +313,10 @@ WARNING: relation "tbl_nn" must have a primary key or not-null unique keys WARNING: relation "tbl_uk" must have a primary key or not-null unique keys -- => WARNING \! pg_repack --dbname=contrib_regression --table=tbl_nn_uk +INFO: repacking table "tbl_nn_uk" -- => OK \! pg_repack --dbname=contrib_regression --table=tbl_pk_uk +INFO: repacking table "tbl_pk_uk" -- => OK \! pg_repack --dbname=contrib_regression --table=tbl_nn_puk WARNING: relation "tbl_nn_puk" must have a primary key or not-null unique keys @@ -326,6 +333,7 @@ SELECT repack.get_order_by('issue3_1_idx'::regclass::oid, 'issue3_1'::regclass:: (1 row) \! pg_repack --dbname=contrib_regression --table=issue3_1 +INFO: repacking table "issue3_1" CREATE TABLE issue3_2 (col1 int NOT NULL, col2 text NOT NULL); CREATE UNIQUE INDEX issue3_2_idx ON issue3_2 (col1 DESC, col2 text_pattern_ops); SELECT repack.get_order_by('issue3_2_idx'::regclass::oid, 'issue3_2'::regclass::oid); @@ -335,6 +343,7 @@ SELECT repack.get_order_by('issue3_2_idx'::regclass::oid, 'issue3_2'::regclass:: (1 row) \! pg_repack --dbname=contrib_regression --table=issue3_2 +INFO: repacking table "issue3_2" CREATE TABLE issue3_3 (col1 int NOT NULL, col2 text NOT NULL); CREATE UNIQUE INDEX issue3_3_idx ON issue3_3 (col1 DESC, col2 DESC); SELECT repack.get_order_by('issue3_3_idx'::regclass::oid, 'issue3_3'::regclass::oid); @@ -344,6 +353,7 @@ SELECT repack.get_order_by('issue3_3_idx'::regclass::oid, 'issue3_3'::regclass:: (1 row) \! pg_repack --dbname=contrib_regression --table=issue3_3 +INFO: repacking table "issue3_3" CREATE TABLE issue3_4 (col1 int NOT NULL, col2 text NOT NULL); CREATE UNIQUE INDEX issue3_4_idx ON issue3_4 (col1 NULLS FIRST, col2 text_pattern_ops DESC NULLS LAST); SELECT repack.get_order_by('issue3_4_idx'::regclass::oid, 'issue3_4'::regclass::oid); @@ -353,6 +363,7 @@ SELECT repack.get_order_by('issue3_4_idx'::regclass::oid, 'issue3_4'::regclass:: (1 row) \! pg_repack --dbname=contrib_regression --table=issue3_4 +INFO: repacking table "issue3_4" CREATE TABLE issue3_5 (col1 int NOT NULL, col2 text NOT NULL); CREATE UNIQUE INDEX issue3_5_idx ON issue3_5 (col1 DESC NULLS FIRST, col2 COLLATE "POSIX" DESC); SELECT repack.get_order_by('issue3_5_idx'::regclass::oid, 'issue3_5'::regclass::oid); @@ -362,3 +373,4 @@ SELECT repack.get_order_by('issue3_5_idx'::regclass::oid, 'issue3_5'::regclass:: (1 row) \! pg_repack --dbname=contrib_regression --table=issue3_5 +INFO: repacking table "issue3_5" diff --git a/bin/expected/tablespace.out b/bin/expected/tablespace.out index e6daf5c..cf9a93e 100644 --- a/bin/expected/tablespace.out +++ b/bin/expected/tablespace.out @@ -18,6 +18,7 @@ INSERT INTO testts1 (data) values ('b'); INSERT INTO testts1 (data) values ('c'); -- can move the tablespace from default \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts +INFO: repacking table "testts1" SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace WHERE relname ~ '^testts1' @@ -37,6 +38,7 @@ SELECT * from testts1 order by id; -- tablespace stays where it is \! pg_repack --dbname=contrib_regression --no-order --table=testts1 +INFO: repacking table "testts1" SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace WHERE relname ~ '^testts1' @@ -48,6 +50,7 @@ ORDER BY relname; -- can move the ts back to default \! pg_repack --dbname=contrib_regression --no-order --table=testts1 -s pg_default +INFO: repacking table "testts1" SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace WHERE relname ~ '^testts1' @@ -58,6 +61,7 @@ ORDER BY relname; -- can move the table together with the indexes \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts --moveidx +INFO: repacking table "testts1" SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace WHERE relname ~ '^testts1' diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 681fa44..b515924 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -918,6 +918,8 @@ repack_one_table(const repack_table *table, const char *orderby) initStringInfo(&sql); + elog(INFO, "repacking table \"%s\"", table->target_name); + elog(DEBUG2, "---- repack_one_table ----"); elog(DEBUG2, "target_name : %s", table->target_name); elog(DEBUG2, "target_oid : %u", table->target_oid);