From 6710e514db8f590e4c9d980300f7642bc146502a Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 21 Feb 2013 17:10:12 +0000 Subject: [PATCH 01/33] Added --namespace option to set the namespace of repacked tables Bumped version number to enforce extension re-creation as the SQL has been modified. Current limitations: - Check for namespace existence: on error temp objects are left around - What happens to the indexes? - Tests needed. - Should the default be the GUC default_tablespace instead of pg_default? This is actually an original pg_repack shortcoming, not a regression. --- META.json | 4 +-- bin/pg_repack.c | 80 +++++++++++++++++++++++++++++++------------- lib/pg_repack.sql.in | 4 ++- 3 files changed, 62 insertions(+), 26 deletions(-) diff --git a/META.json b/META.json index ca4c4b1..454e3a5 100644 --- a/META.json +++ b/META.json @@ -2,7 +2,7 @@ "name": "pg_repack", "abstract": "PostgreSQL module for data reorganization", "description": "Reorganize tables in PostgreSQL databases with minimal locks", - "version": "1.2dev0", + "version": "1.2dev1", "maintainer": [ "Josh Kupershmidt ", "Daniele Varrazzo " @@ -13,7 +13,7 @@ "provides": { "pg_repack": { "file": "lib/pg_repack.sql", - "version": "1.2dev0", + "version": "1.2dev1", "abstract": "Reorganize tables in PostgreSQL databases with minimal locks" } }, diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 78ed909..41240d9 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -197,6 +197,7 @@ static bool alldb = false; static bool noorder = false; static SimpleStringList table_list = {NULL, NULL}; static char *orderby = NULL; +static char *tablespace = NULL; static int wait_timeout = 60; /* in seconds */ static int jobs = 0; /* number of concurrent worker conns. */ @@ -214,6 +215,7 @@ static pgut_option options[] = { 'l', 't', "table", &table_list }, { 'b', 'n', "no-order", &noorder }, { 's', 'o', "order-by", &orderby }, + { 's', 's', "tablespace", &tablespace }, { 'i', 'T', "wait-timeout", &wait_timeout }, { 'B', 'Z', "no-analyze", &analyze }, { 'i', 'j', "jobs", &jobs }, @@ -360,10 +362,15 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) StringInfoData sql; SimpleStringListCell *cell; const char **params = NULL; - size_t num_params = simple_string_list_size(table_list); + int iparam = 0; + size_t num_tables; + size_t num_params; - if (num_params) - params = pgut_malloc(num_params * sizeof(char *)); + num_tables = simple_string_list_size(table_list); + + /* 1st param is the user-specified tablespace */ + num_params = num_tables + 1; + params = pgut_malloc(num_params * sizeof(char *)); initStringInfo(&sql); @@ -442,29 +449,46 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) command("SET client_min_messages = warning", 0, NULL); /* acquire target tables */ - appendStringInfoString(&sql, "SELECT * FROM repack.tables WHERE "); - if (num_params) + appendStringInfoString(&sql, + "SELECT t.*," + " coalesce(v.tablespace, t.tablespace_orig) as tablespace_dest" + " FROM repack.tables t, " + " (VALUES (quote_ident($1::text))) as v (tablespace)" + " WHERE "); + + params[iparam++] = tablespace; + if (num_tables) { appendStringInfoString(&sql, "("); - for (i = 0, cell = table_list.head; cell; cell = cell->next, i++) + for (cell = table_list.head; cell; cell = cell->next) { /* Construct table name placeholders to be used by PQexecParams */ - appendStringInfo(&sql, "relid = $%d::regclass", i + 1); - params[i] = cell->val; + appendStringInfo(&sql, "relid = $%d::regclass", iparam + 1); + params[iparam++] = cell->val; if (cell->next) appendStringInfoString(&sql, " OR "); } appendStringInfoString(&sql, ")"); - res = execute_elevel(sql.data, (int) num_params, params, DEBUG2); } else { appendStringInfoString(&sql, "pkid IS NOT NULL"); if (!orderby) appendStringInfoString(&sql, " AND ckid IS NOT NULL"); - res = execute_elevel(sql.data, 0, NULL, DEBUG2); } + /* double check the parameters array is sane */ + if (iparam != num_params) + { + if (errbuf) + snprintf(errbuf, errsize, + "internal error: bad parameters count: %i instead of %zi", + iparam, num_params); + goto cleanup; + } + + res = execute_elevel(sql.data, (int) num_params, params, DEBUG2); + /* on error skip the database */ if (PQresultStatus(res) != PGRES_TUPLES_OK) { @@ -489,7 +513,9 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) for (i = 0; i < num; i++) { repack_table table; - const char *create_table; + const char *create_table_1; + const char *create_table_2; + const char *tablespace; const char *ckey; int c = 0; @@ -512,13 +538,24 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) table.create_trigger = getstr(res, i, c++); table.enable_trigger = getstr(res, i, c++); - create_table = getstr(res, i, c++); + create_table_1 = getstr(res, i, c++); + tablespace = getstr(res, i, c++); /* to be clobbered */ + create_table_2 = getstr(res, i, c++); table.drop_columns = getstr(res, i, c++); table.delete_log = getstr(res, i, c++); table.lock_table = getstr(res, i, c++); ckey = getstr(res, i, c++); + table.sql_peek = getstr(res, i, c++); + table.sql_insert = getstr(res, i, c++); + table.sql_delete = getstr(res, i, c++); + table.sql_update = getstr(res, i, c++); + table.sql_pop = getstr(res, i, c++); + tablespace = getstr(res, i, c++); resetStringInfo(&sql); + appendStringInfoString(&sql, create_table_1); + appendStringInfoString(&sql, tablespace); + appendStringInfoString(&sql, create_table_2); if (!orderby) { /* CLUSTER mode */ @@ -529,27 +566,23 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) errmsg("relation \"%s\" has no cluster key", table.target_name))); continue; } - appendStringInfo(&sql, "%s ORDER BY %s", create_table, ckey); - table.create_table = sql.data; + appendStringInfoString(&sql, " ORDER BY "); + appendStringInfoString(&sql, ckey); + table.create_table = sql.data; } else if (!orderby[0]) { /* VACUUM FULL mode */ - table.create_table = create_table; + table.create_table = sql.data; } else { /* User specified ORDER BY */ - appendStringInfo(&sql, "%s ORDER BY %s", create_table, orderby); - table.create_table = sql.data; + appendStringInfoString(&sql, " ORDER BY "); + appendStringInfoString(&sql, orderby); + table.create_table = sql.data; } - table.sql_peek = getstr(res, i, c++); - table.sql_insert = getstr(res, i, c++); - table.sql_delete = getstr(res, i, c++); - table.sql_update = getstr(res, i, c++); - table.sql_pop = getstr(res, i, c++); - repack_one_table(&table, orderby); } ret = true; @@ -1430,6 +1463,7 @@ pgut_help(bool details) printf(" -n, --no-order do vacuum full instead of cluster\n"); printf(" -o, --order-by=COLUMNS order by columns instead of cluster keys\n"); printf(" -t, --table=TABLE repack specific table only\n"); + printf(" -s, --tablespace=TABLESPC move repacked tables to a new tablespace\n"); printf(" -T, --wait-timeout=SECS timeout to cancel other backends on conflict\n"); printf(" -Z, --no-analyze don't analyze at end\n"); } diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index 3f78d19..e60c80d 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -179,7 +179,9 @@ CREATE VIEW repack.tables AS 'CREATE TABLE repack.log_' || R.oid || ' (id bigserial PRIMARY KEY, pk repack.pk_' || R.oid || ', row ' || repack.oid2text(R.oid) || ')' AS create_log, repack.get_create_trigger(R.oid, PK.indexrelid) AS create_trigger, repack.get_enable_trigger(R.oid) as enable_trigger, - 'CREATE TABLE repack.table_' || R.oid || ' WITH (' || array_to_string(array_append(R.reloptions, 'oids=' || CASE WHEN R.relhasoids THEN 'true' ELSE 'false' END), ',') || ') TABLESPACE ' || coalesce(quote_ident(S.spcname), 'pg_default') || ' AS SELECT ' || repack.get_columns_for_create_as(R.oid) || ' FROM ONLY ' || repack.oid2text(R.oid) AS create_table, + 'CREATE TABLE repack.table_' || R.oid || ' WITH (' || array_to_string(array_append(R.reloptions, 'oids=' || CASE WHEN R.relhasoids THEN 'true' ELSE 'false' END), ',') || ') TABLESPACE ' AS create_table_1, + coalesce(quote_ident(S.spcname), 'pg_default') tablespace_orig, + ' AS SELECT ' || repack.get_columns_for_create_as(R.oid) || ' FROM ONLY ' || repack.oid2text(R.oid) AS create_table_2, repack.get_drop_columns(R.oid, 'repack.table_' || R.oid) AS drop_columns, 'DELETE FROM repack.log_' || R.oid AS delete_log, 'LOCK TABLE ' || repack.oid2text(R.oid) || ' IN ACCESS EXCLUSIVE MODE' AS lock_table, From 6488ecabd25d2921ba693177dd25614be5df64d5 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 16 Apr 2013 18:32:46 +0100 Subject: [PATCH 02/33] Added --moveidx command line option The option is only parsed, not implemented yet. --- bin/pg_repack.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 41240d9..c3b8b16 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -198,6 +198,7 @@ static bool noorder = false; static SimpleStringList table_list = {NULL, NULL}; static char *orderby = NULL; static char *tablespace = NULL; +static bool moveidx = false; static int wait_timeout = 60; /* in seconds */ static int jobs = 0; /* number of concurrent worker conns. */ @@ -216,6 +217,7 @@ static pgut_option options[] = { 'b', 'n', "no-order", &noorder }, { 's', 'o', "order-by", &orderby }, { 's', 's', "tablespace", &tablespace }, + { 'b', 'S', "moveidx", &moveidx }, { 'i', 'T', "wait-timeout", &wait_timeout }, { 'B', 'Z', "no-analyze", &analyze }, { 'i', 'j', "jobs", &jobs }, @@ -256,6 +258,12 @@ main(int argc, char *argv[]) errmsg("%s", errbuf))); } + if (moveidx && tablespace == NULL) + { + ereport(ERROR, + (errcode(EINVAL), + errmsg("cannot specify --moveidx (-S) without --tablespace (-s)"))); + } return 0; } @@ -1464,6 +1472,7 @@ pgut_help(bool details) printf(" -o, --order-by=COLUMNS order by columns instead of cluster keys\n"); printf(" -t, --table=TABLE repack specific table only\n"); printf(" -s, --tablespace=TABLESPC move repacked tables to a new tablespace\n"); + printf(" -S, --moveidx move repacked indexes to TABLESPC too\n"); printf(" -T, --wait-timeout=SECS timeout to cancel other backends on conflict\n"); printf(" -Z, --no-analyze don't analyze at end\n"); } From c542bf264100756580a6d79b2b2c942921f7320a Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 16 Apr 2013 18:33:23 +0100 Subject: [PATCH 03/33] Added tests for the namespace change feature Currently not passing as --moveidx not implemented yet. --- bin/Makefile | 2 +- bin/expected/tablespace.out | 70 +++++++++++++++++++++++++++++++++++++ bin/sql/tablespace.sql | 50 ++++++++++++++++++++++++++ 3 files changed, 121 insertions(+), 1 deletion(-) create mode 100644 bin/expected/tablespace.out create mode 100644 bin/sql/tablespace.sql diff --git a/bin/Makefile b/bin/Makefile index 3e9342b..24ae1cd 100644 --- a/bin/Makefile +++ b/bin/Makefile @@ -8,7 +8,7 @@ SRCS = pg_repack.c pgut/pgut.c pgut/pgut-fe.c OBJS = $(SRCS:.c=.o) PROGRAM = pg_repack -REGRESS = init repack +REGRESS = init repack tablespace EXTRA_CLEAN = sql/init-$(MAJORVERSION).sql sql/init.sql diff --git a/bin/expected/tablespace.out b/bin/expected/tablespace.out new file mode 100644 index 0000000..9fa432a --- /dev/null +++ b/bin/expected/tablespace.out @@ -0,0 +1,70 @@ +SET client_min_messages = warning; +-- +-- Tablespace features tests +-- +-- Note: in order to pass this test you must create a tablespace called 'testts' +-- +SELECT spcname FROM pg_tablespace WHERE spcname = 'testts'; + spcname +--------- + testts +(1 row) + +-- If the query above failed you must create the 'testts' tablespace; +CREATE TABLE testts1 (id serial primary key, data text); +INSERT INTO testts1 (data) values ('a'); +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 +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1'; + relname | spcname +---------+--------- + testts1 | testts +(1 row) + +SELECT * from testts1 order by id; + id | data +----+------ + 1 | a + 2 | b + 3 | c +(3 rows) + +-- tablespace stays where it is +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1'; + relname | spcname +---------+--------- + testts1 | testts +(1 row) + +-- can move the ts back to default +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 -s pg_default +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1'; + relname | spcname +---------+--------- +(0 rows) + +-- can move the table together with the indexes +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace pg_default --moveidx +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1'; + relname | spcname +--------------+--------- + testts1 | testts + testts1_pkey | testts +(2 rows) + +-- can't specify --moveidx without --tablespace +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --moveidx +ERROR: cannot specify --moveidx (-S) without --tablespace (-s) +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 -S +ERROR: cannot specify --moveidx (-S) without --tablespace (-s) diff --git a/bin/sql/tablespace.sql b/bin/sql/tablespace.sql new file mode 100644 index 0000000..291b6a6 --- /dev/null +++ b/bin/sql/tablespace.sql @@ -0,0 +1,50 @@ +SET client_min_messages = warning; + +-- +-- Tablespace features tests +-- +-- Note: in order to pass this test you must create a tablespace called 'testts' +-- + +SELECT spcname FROM pg_tablespace WHERE spcname = 'testts'; +-- If the query above failed you must create the 'testts' tablespace; + +CREATE TABLE testts1 (id serial primary key, data text); +INSERT INTO testts1 (data) values ('a'); +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 + +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1'; + +SELECT * from testts1 order by id; + +-- tablespace stays where it is +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 + +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1'; + +-- can move the ts back to default +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 -s pg_default + +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1'; + +-- can move the table together with the indexes +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace pg_default --moveidx + +SELECT relname, spcname +FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace +WHERE relname ~ '^testts1'; + +-- can't specify --moveidx without --tablespace +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --moveidx +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 -S + From 43dfe229c9b611c7e4608d3821ad02cd0936fe30 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 16 Apr 2013 19:14:49 +0100 Subject: [PATCH 04/33] Added check for target tablespace existence --- bin/expected/tablespace.out | 2 +- bin/pg_repack.c | 59 +++++++++++++++++++++++++++++++++---- bin/sql/tablespace.sql | 2 +- 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/bin/expected/tablespace.out b/bin/expected/tablespace.out index 9fa432a..d277122 100644 --- a/bin/expected/tablespace.out +++ b/bin/expected/tablespace.out @@ -53,7 +53,7 @@ WHERE relname ~ '^testts1'; (0 rows) -- can move the table together with the indexes -\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace pg_default --moveidx +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts --moveidx 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 c3b8b16..3acdccd 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -172,6 +172,7 @@ typedef struct repack_index } repack_index; static bool is_superuser(void); +static void check_tablespace(void); static void repack_all_databases(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); @@ -238,6 +239,8 @@ main(int argc, char *argv[]) (errcode(EINVAL), errmsg("too many arguments"))); + check_tablespace(); + if (noorder) orderby = ""; @@ -258,12 +261,6 @@ main(int argc, char *argv[]) errmsg("%s", errbuf))); } - if (moveidx && tablespace == NULL) - { - ereport(ERROR, - (errcode(EINVAL), - errmsg("cannot specify --moveidx (-S) without --tablespace (-s)"))); - } return 0; } @@ -291,6 +288,56 @@ is_superuser(void) return false; } +/* + * Check if the tablespace requested exists. + * + * Raise an exception on error. + */ +void +check_tablespace() +{ + PGresult *res = NULL; + const char *params[1]; + + if (tablespace == NULL) + { + /* nothing to check, but let's see the options */ + if (moveidx) + { + ereport(ERROR, + (errcode(EINVAL), + errmsg("cannot specify --moveidx (-S) without --tablespace (-s)"))); + } + return; + } + + /* check if the tablespace exists */ + reconnect(ERROR); + params[0] = tablespace; + res = execute_elevel( + "select spcname from pg_tablespace where spcname = $1", + 1, params, DEBUG2); + + if (PQresultStatus(res) == PGRES_TUPLES_OK) + { + if (PQntuples(res) == 0) + { + ereport(ERROR, + (errcode(EINVAL), + errmsg("the tablespace \"%s\" doesn't exist", tablespace))); + } + } + else + { + ereport(ERROR, + (errcode(EINVAL), + errmsg("error checking the namespace: %s", + PQerrorMessage(connection)))); + } + + CLEARPGRES(res); +} + /* * Call repack_one_database for each database. diff --git a/bin/sql/tablespace.sql b/bin/sql/tablespace.sql index 291b6a6..c618a4a 100644 --- a/bin/sql/tablespace.sql +++ b/bin/sql/tablespace.sql @@ -38,7 +38,7 @@ FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace WHERE relname ~ '^testts1'; -- can move the table together with the indexes -\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace pg_default --moveidx +\! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts --moveidx SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace From 83fdb2a9e06c46433e51052357af38b547d3c3ca Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 16 Apr 2013 20:17:51 +0100 Subject: [PATCH 05/33] Added implementation for --moveidx Note: if original namespace is "foo bar", repack_indexdef gives a bad result. This is weird as apparently skip_ident can deal with spaces in a quoted identifier. Committing as I'm going home, will deal with that later. --- bin/pg_repack.c | 8 ++++--- lib/pg_repack.sql.in | 4 ++-- lib/repack.c | 50 ++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 53 insertions(+), 9 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 3acdccd..c8291d9 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -681,7 +681,7 @@ static bool rebuild_indexes(const repack_table *table) { PGresult *res; - const char *params[1]; + const char *params[2]; int num_indexes; int i; int num_active_workers; @@ -693,6 +693,7 @@ rebuild_indexes(const repack_table *table) elog(DEBUG2, "---- create indexes ----"); params[0] = utoa(table->target_oid, buffer); + params[1] = moveidx ? tablespace : NULL; /* First, just display a warning message for any invalid indexes * which may be on the table (mostly to match the behavior of 1.1.8). @@ -708,8 +709,9 @@ rebuild_indexes(const repack_table *table) } res = execute("SELECT indexrelid," - " repack.repack_indexdef(indexrelid, indrelid) " - " FROM pg_index WHERE indrelid = $1 AND indisvalid", 1, params); + " repack.repack_indexdef(indexrelid, indrelid, $2) " + " FROM pg_index WHERE indrelid = $1 AND indisvalid", + 2, params); num_indexes = PQntuples(res); diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index 348fa95..7601008 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -207,9 +207,9 @@ CREATE VIEW repack.tables AS AND N.nspname NOT IN ('pg_catalog', 'information_schema') AND N.nspname NOT LIKE E'pg\\_temp\\_%'; -CREATE FUNCTION repack.repack_indexdef(oid, oid) RETURNS text AS +CREATE FUNCTION repack.repack_indexdef(oid, oid, name) RETURNS text AS 'MODULE_PATHNAME', 'repack_indexdef' -LANGUAGE C STABLE STRICT; +LANGUAGE C STABLE; CREATE FUNCTION repack.repack_trigger() RETURNS trigger AS 'MODULE_PATHNAME', 'repack_trigger' diff --git a/lib/repack.c b/lib/repack.c index 1bfec43..d315bbf 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -618,20 +618,62 @@ repack_get_order_by(PG_FUNCTION_ARGS) * * @param index Oid of target index. * @param table Oid of table of the index. + * @param tablespace Namespace for the index. If NULL keep the original. * @retval Create index DDL for temp table. */ Datum repack_indexdef(PG_FUNCTION_ARGS) { - Oid index = PG_GETARG_OID(0); - Oid table = PG_GETARG_OID(1); + Oid index; + Oid table; + Name tablespace = NULL; IndexDef stmt; StringInfoData str; + if (PG_ARGISNULL(0) || PG_ARGISNULL(1)) + PG_RETURN_NULL(); + + index = PG_GETARG_OID(0); + table = PG_GETARG_OID(1); + + if (!PG_ARGISNULL(2)) + tablespace = PG_GETARG_NAME(2); + parse_indexdef(&stmt, index, table); + initStringInfo(&str); - appendStringInfo(&str, "%s index_%u ON repack.table_%u USING %s (%s)%s", - stmt.create, index, table, stmt.type, stmt.columns, stmt.options); + appendStringInfo(&str, "%s index_%u ON repack.table_%u USING %s (%s)", + stmt.create, index, table, stmt.type, stmt.columns); + + /* Replace the tablespace in the index options */ + if (tablespace == NULL) + { + /* tablespace is just fine */ + appendStringInfoString(&str, stmt.options); + } + else + { + const char *pos; + if (NULL == (pos = strstr(stmt.options, " TABLESPACE "))) + { + /* tablespace is to append */ + appendStringInfoString(&str, " TABLESPACE "); + appendStringInfoString(&str, NameStr(*tablespace)); + } + else + { + char *tmp; + + /* tablespace is to replace */ + tmp = skip_const(index, stmt.options, " TABLESPACE ", NULL); + appendStringInfoString(&str, stmt.options); + appendStringInfoString(&str, NameStr(*tablespace)); + /* FIXME: not working if original ts has a space. But skip_ident + * should deal with that. Stupid mistake somewhere? */ + tmp = skip_ident(index, tmp); + appendStringInfoString(&str, tmp); + } + } PG_RETURN_TEXT_P(cstring_to_text(str.data)); } From d98a14bb559ed41e74417b75564d287123372a2a Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 16 Apr 2013 23:23:26 +0100 Subject: [PATCH 06/33] Fixed index definition tokenization In the previous commit skip_const was going ahead the space thus removing the starting quote. Also fixed (and tested) trailing part after the tablespace name, e.g. the WHERE clause. --- bin/expected/tablespace.out | 24 +++++++++++++++--------- bin/sql/tablespace.sql | 13 +++++++++---- lib/repack.c | 15 ++++++--------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/bin/expected/tablespace.out b/bin/expected/tablespace.out index d277122..e6daf5c 100644 --- a/bin/expected/tablespace.out +++ b/bin/expected/tablespace.out @@ -12,6 +12,7 @@ SELECT spcname FROM pg_tablespace WHERE spcname = 'testts'; -- If the query above failed you must create the 'testts' tablespace; CREATE TABLE testts1 (id serial primary key, data text); +CREATE INDEX testts1_partial_idx on testts1 (id) where (id > 0); INSERT INTO testts1 (data) values ('a'); INSERT INTO testts1 (data) values ('b'); INSERT INTO testts1 (data) values ('c'); @@ -19,7 +20,8 @@ INSERT INTO testts1 (data) values ('c'); \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace -WHERE relname ~ '^testts1'; +WHERE relname ~ '^testts1' +ORDER BY relname; relname | spcname ---------+--------- testts1 | testts @@ -37,7 +39,8 @@ SELECT * from testts1 order by id; \! pg_repack --dbname=contrib_regression --no-order --table=testts1 SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace -WHERE relname ~ '^testts1'; +WHERE relname ~ '^testts1' +ORDER BY relname; relname | spcname ---------+--------- testts1 | testts @@ -47,7 +50,8 @@ WHERE relname ~ '^testts1'; \! pg_repack --dbname=contrib_regression --no-order --table=testts1 -s pg_default SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace -WHERE relname ~ '^testts1'; +WHERE relname ~ '^testts1' +ORDER BY relname; relname | spcname ---------+--------- (0 rows) @@ -56,12 +60,14 @@ WHERE relname ~ '^testts1'; \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts --moveidx SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace -WHERE relname ~ '^testts1'; - relname | spcname ---------------+--------- - testts1 | testts - testts1_pkey | testts -(2 rows) +WHERE relname ~ '^testts1' +ORDER BY relname; + relname | spcname +---------------------+--------- + testts1 | testts + testts1_partial_idx | testts + testts1_pkey | testts +(3 rows) -- can't specify --moveidx without --tablespace \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --moveidx diff --git a/bin/sql/tablespace.sql b/bin/sql/tablespace.sql index c618a4a..73400e7 100644 --- a/bin/sql/tablespace.sql +++ b/bin/sql/tablespace.sql @@ -10,6 +10,7 @@ SELECT spcname FROM pg_tablespace WHERE spcname = 'testts'; -- If the query above failed you must create the 'testts' tablespace; CREATE TABLE testts1 (id serial primary key, data text); +CREATE INDEX testts1_partial_idx on testts1 (id) where (id > 0); INSERT INTO testts1 (data) values ('a'); INSERT INTO testts1 (data) values ('b'); INSERT INTO testts1 (data) values ('c'); @@ -19,7 +20,8 @@ INSERT INTO testts1 (data) values ('c'); SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace -WHERE relname ~ '^testts1'; +WHERE relname ~ '^testts1' +ORDER BY relname; SELECT * from testts1 order by id; @@ -28,21 +30,24 @@ SELECT * from testts1 order by id; SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace -WHERE relname ~ '^testts1'; +WHERE relname ~ '^testts1' +ORDER BY relname; -- can move the ts back to default \! pg_repack --dbname=contrib_regression --no-order --table=testts1 -s pg_default SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace -WHERE relname ~ '^testts1'; +WHERE relname ~ '^testts1' +ORDER BY relname; -- can move the table together with the indexes \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts --moveidx SELECT relname, spcname FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace -WHERE relname ~ '^testts1'; +WHERE relname ~ '^testts1' +ORDER BY relname; -- can't specify --moveidx without --tablespace \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --moveidx diff --git a/lib/repack.c b/lib/repack.c index d315bbf..19cb089 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -653,8 +653,7 @@ repack_indexdef(PG_FUNCTION_ARGS) } else { - const char *pos; - if (NULL == (pos = strstr(stmt.options, " TABLESPACE "))) + if (NULL == strstr(stmt.options, "TABLESPACE")) { /* tablespace is to append */ appendStringInfoString(&str, " TABLESPACE "); @@ -662,16 +661,14 @@ repack_indexdef(PG_FUNCTION_ARGS) } else { - char *tmp; - /* tablespace is to replace */ - tmp = skip_const(index, stmt.options, " TABLESPACE ", NULL); + char *tmp; + tmp = skip_const(index, stmt.options, " TABLESPACE", NULL); appendStringInfoString(&str, stmt.options); - appendStringInfoString(&str, NameStr(*tablespace)); - /* FIXME: not working if original ts has a space. But skip_ident - * should deal with that. Stupid mistake somewhere? */ + appendStringInfo(&str, " %s", NameStr(*tablespace)); tmp = skip_ident(index, tmp); - appendStringInfoString(&str, tmp); + if (*tmp) + appendStringInfo(&str, " %s", tmp); } } From b6bd078e92df06da63ade3479f16989b70e95f1b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 16 Apr 2013 23:39:51 +0100 Subject: [PATCH 07/33] Added documentation for --tablespace and --moveidx --- doc/pg_repack.rst | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 0e1e66d..35b807c 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -121,6 +121,8 @@ Options: -n, --no-order do vacuum full instead of cluster -o, --order-by=COLUMNS order by columns instead of cluster keys -t, --table=TABLE repack specific table only + -s, --tablespace=TABLESPC move repacked tables to a new tablespace + -S, --moveidx move repacked indexes to TABLESPC too -T, --wait-timeout=SECS timeout to cancel other backends on conflict -Z, --no-analyze don't analyze at end @@ -162,6 +164,15 @@ target tables or databases. Reorganize the specified table only. By default, all eligible tables in the target databases are reorganized. +``-s TABLESPC``, ``--tablespace=TABLESPC`` + Move the repacked tables to the specified tablespace: essentially an + online version of ``ALTER TABLE ... SET TABLESPACE``. The tables indexes + are left on the original tablespace unless ``--moveidx`` is specified too. + +``-S``, ``--moveidx`` + Move the indexes too of the repacked tables to the tablespace specified + by the option ``--tablespace``. + ``-T SECS``, ``--wait-timeout=SECS`` pg_repack needs to take an exclusive lock at the end of the reorganization. This setting controls how many seconds pg_repack will @@ -175,6 +186,7 @@ target tables or databases. Disable ANALYZE after the reorganization. If not specified, run ANALYZE after the reorganization. + Connection Options ^^^^^^^^^^^^^^^^^^ @@ -399,6 +411,10 @@ and the original one. Releases -------- +* pg_repack 1.2 + + * Added --tablespace and --moveidx options to perform online SET TABLESPACE. + * pg_repack 1.1.8 * Added support for PostgreSQL 9.2. From 127d5cbfb27867385f320372a7130840ec4f886c Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Tue, 16 Apr 2013 23:50:52 +0100 Subject: [PATCH 08/33] Added missing entries to the pg_repack 1.2 changes --- doc/pg_repack.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 0e1e66d..4509c36 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -399,6 +399,12 @@ and the original one. Releases -------- +* pg_repack 1.2 + + * Added --jobs option for parallel operation. + * Bugfix: correctly handle key indexes with options such as DESC, NULL + FIRST/LAST, COLLATE (pg_repack issue #3). + * pg_repack 1.1.8 * Added support for PostgreSQL 9.2. From 1d62d8d0c5a28b56421c80259130d4468c357b1e Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 00:53:54 +0100 Subject: [PATCH 09/33] More helpful error messages in case of conflicting triggers Closes issue #5. --- bin/pg_repack.c | 31 ++++++++++++++++++++++++++++--- doc/pg_repack.rst | 15 +++++++++------ lib/pg_repack.sql.in | 1 + 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 78ed909..ec325ed 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -886,10 +886,35 @@ repack_one_table(const repack_table *table, const char *orderby) res = execute("SELECT repack.conflicted_triggers($1)", 1, params); if (PQntuples(res) > 0) { - ereport(WARNING, - (errcode(E_PG_COMMAND), - errmsg("trigger %s conflicted for %s", + if (0 == strcmp("z_repack_trigger", PQgetvalue(res, 0, 0))) + { + ereport(WARNING, + (errcode(E_PG_COMMAND), + errmsg("the table \"%s\" has already a trigger called \"%s\"", + table->target_name, PQgetvalue(res, 0, 0)), + errdetail( + "The trigger was probably installed during a previous" + " attempt to run pg_repack on the table which was" + " interrupted and for some reason failed to clean up" + " the temporary objects. Please drop the trigger or drop" + " and recreate the pg_repack extension altogether" + " to remove all the temporary objects left over."))); + } + else + { + ereport(WARNING, + (errcode(E_PG_COMMAND), + errmsg("trigger \"%s\" conflicting on table \"%s\"", + PQgetvalue(res, 0, 0), table->target_name), + errdetail( + "The trigger \"z_repack_trigger\" must be the last of the" + " BEFORE triggers to fire on the table (triggers fire in" + " alphabetical order). Please rename the trigger so that" + " it sorts before \"z_repack_trigger\": you can use" + " \"ALTER TRIGGER %s ON %s RENAME TO newname\".", PQgetvalue(res, 0, 0), table->target_name))); + } + have_error = true; goto cleanup; } diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 4509c36..9692081 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -329,21 +329,23 @@ ERROR: permission denied for schema repack pg_repack must be executed by a superuser. -pg_repack: query failed: ERROR: trigger "z_repack_trigger" for relation "tbl" already exists - The target table has already a trigger named ``z_repack_trigger``. This - is probably caused by a previous failed attempt to run pg_repack on the - table, which for some reason failed to clean up the temporary object. +WARNING: the table "tbl" has already a trigger called z_repack_trigger + The trigger was probably installed during a previous attempt to run + pg_repack on the table which was interrupted and for some reason failed + to clean up the temporary objects. You can remove all the temporary objects by dropping and re-creating the extension: see the installation_ section for the details. -pg_repack: trigger conflicted for tbl +WARNING: trigger "trg" conflicting on table "tbl" The target table has a trigger whose name follows ``z_repack_trigger`` in alphabetical order. The ``z_repack_trigger`` should be the last BEFORE trigger to fire. Please rename your trigger to that it sorts alphabetically before - pg_repack's one. + pg_repack's one; you can use:: + + ALTER TRIGGER zzz_my_trigger ON sometable RENAME TO yyy_my_trigger; Restrictions @@ -402,6 +404,7 @@ Releases * pg_repack 1.2 * Added --jobs option for parallel operation. + * More helpful error messages. * Bugfix: correctly handle key indexes with options such as DESC, NULL FIRST/LAST, COLLATE (pg_repack issue #3). diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index 1767938..cfc9512 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -217,6 +217,7 @@ CREATE FUNCTION repack.conflicted_triggers(oid) RETURNS SETOF name AS $$ SELECT tgname FROM pg_trigger WHERE tgrelid = $1 AND tgname >= 'z_repack_trigger' + ORDER BY tgname; $$ LANGUAGE sql STABLE STRICT; From 8efbd9e1c63fd1b1dac85595701ccafd94ff823f Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 01:42:22 +0100 Subject: [PATCH 10/33] Imply --no-order for non-clustered tables --no-order can still be specified to VACUUM FULL a clustered tables (not so useful I guess...) Fixes issue #6. --- bin/expected/repack.out | 22 +++++++++++----------- bin/pg_repack.c | 23 ++++++++++++----------- bin/sql/repack.sql | 22 +++++++++++----------- doc/pg_repack.rst | 12 +++++++----- 4 files changed, 41 insertions(+), 38 deletions(-) diff --git a/bin/expected/repack.out b/bin/expected/repack.out index 222e270..5026659 100644 --- a/bin/expected/repack.out +++ b/bin/expected/repack.out @@ -114,7 +114,7 @@ SELECT * FROM tbl_with_dropped_toast; -- -- do repack -- -\! pg_repack --dbname=contrib_regression --no-order +\! pg_repack --dbname=contrib_regression --table=tbl_badindex WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON tbl_badindex USING btree (n) \! pg_repack --dbname=contrib_regression \! pg_repack --dbname=contrib_regression --table=tbl_cluster @@ -301,17 +301,17 @@ CREATE TABLE tbl_nn_uk (col1 int NOT NULL, col2 int NOT NULL, UNIQUE(col1, col2) CREATE TABLE tbl_pk_uk (col1 int NOT NULL, col2 int NOT NULL, PRIMARY KEY(col1, col2), UNIQUE(col2, col1)); CREATE TABLE tbl_nn_puk (col1 int NOT NULL, col2 int NOT NULL); CREATE UNIQUE INDEX tbl_nn_puk_pcol1_idx ON tbl_nn_puk(col1) WHERE col1 < 10; -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_nn +\! pg_repack --dbname=contrib_regression --table=tbl_nn WARNING: relation "tbl_nn" must have a primary key or not-null unique keys -- => WARNING -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_uk +\! pg_repack --dbname=contrib_regression --table=tbl_uk WARNING: relation "tbl_uk" must have a primary key or not-null unique keys -- => WARNING -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_nn_uk +\! pg_repack --dbname=contrib_regression --table=tbl_nn_uk -- => OK -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_pk_uk +\! pg_repack --dbname=contrib_regression --table=tbl_pk_uk -- => OK -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_nn_puk +\! pg_repack --dbname=contrib_regression --table=tbl_nn_puk WARNING: relation "tbl_nn_puk" must have a primary key or not-null unique keys -- => WARNING -- @@ -325,7 +325,7 @@ SELECT repack.get_order_by('issue3_1_idx'::regclass::oid, 'issue3_1'::regclass:: col1, col2 DESC (1 row) -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_1 +\! pg_repack --dbname=contrib_regression --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); @@ -334,7 +334,7 @@ SELECT repack.get_order_by('issue3_2_idx'::regclass::oid, 'issue3_2'::regclass:: col1 DESC, col2 USING ~<~ (1 row) -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_2 +\! pg_repack --dbname=contrib_regression --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); @@ -343,7 +343,7 @@ SELECT repack.get_order_by('issue3_3_idx'::regclass::oid, 'issue3_3'::regclass:: col1 DESC, col2 DESC (1 row) -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_3 +\! pg_repack --dbname=contrib_regression --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); @@ -352,7 +352,7 @@ SELECT repack.get_order_by('issue3_4_idx'::regclass::oid, 'issue3_4'::regclass:: col1 NULLS FIRST, col2 DESC USING ~<~ NULLS LAST (1 row) -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_4 +\! pg_repack --dbname=contrib_regression --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); @@ -361,4 +361,4 @@ SELECT repack.get_order_by('issue3_5_idx'::regclass::oid, 'issue3_5'::regclass:: col1 DESC, col2 COLLATE "POSIX" DESC (1 row) -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_5 +\! pg_repack --dbname=contrib_regression --table=issue3_5 diff --git a/bin/pg_repack.c b/bin/pg_repack.c index ec325ed..3460379 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -521,27 +521,28 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) resetStringInfo(&sql); if (!orderby) { - /* CLUSTER mode */ - if (ckey == NULL) + if (ckey != NULL) { - ereport(WARNING, - (errcode(E_PG_COMMAND), - errmsg("relation \"%s\" has no cluster key", table.target_name))); - continue; + /* CLUSTER mode */ + appendStringInfo(&sql, "%s ORDER BY %s", create_table, ckey); + table.create_table = sql.data; + } + else + { + /* VACUUM FULL mode (non-clustered tables) */ + table.create_table = create_table; } - appendStringInfo(&sql, "%s ORDER BY %s", create_table, ckey); - table.create_table = sql.data; } else if (!orderby[0]) { - /* VACUUM FULL mode */ - table.create_table = create_table; + /* VACUUM FULL mode (for clustered tables too) */ + table.create_table = create_table; } else { /* User specified ORDER BY */ appendStringInfo(&sql, "%s ORDER BY %s", create_table, orderby); - table.create_table = sql.data; + table.create_table = sql.data; } table.sql_peek = getstr(res, i, c++); diff --git a/bin/sql/repack.sql b/bin/sql/repack.sql index 746baff..b3a4a0e 100644 --- a/bin/sql/repack.sql +++ b/bin/sql/repack.sql @@ -120,7 +120,7 @@ SELECT * FROM tbl_with_dropped_toast; -- do repack -- -\! pg_repack --dbname=contrib_regression --no-order +\! pg_repack --dbname=contrib_regression --table=tbl_badindex \! pg_repack --dbname=contrib_regression \! pg_repack --dbname=contrib_regression --table=tbl_cluster @@ -177,15 +177,15 @@ CREATE TABLE tbl_nn_uk (col1 int NOT NULL, col2 int NOT NULL, UNIQUE(col1, col2) CREATE TABLE tbl_pk_uk (col1 int NOT NULL, col2 int NOT NULL, PRIMARY KEY(col1, col2), UNIQUE(col2, col1)); CREATE TABLE tbl_nn_puk (col1 int NOT NULL, col2 int NOT NULL); CREATE UNIQUE INDEX tbl_nn_puk_pcol1_idx ON tbl_nn_puk(col1) WHERE col1 < 10; -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_nn +\! pg_repack --dbname=contrib_regression --table=tbl_nn -- => WARNING -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_uk +\! pg_repack --dbname=contrib_regression --table=tbl_uk -- => WARNING -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_nn_uk +\! pg_repack --dbname=contrib_regression --table=tbl_nn_uk -- => OK -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_pk_uk +\! pg_repack --dbname=contrib_regression --table=tbl_pk_uk -- => OK -\! pg_repack --dbname=contrib_regression --no-order --table=tbl_nn_puk +\! pg_repack --dbname=contrib_regression --table=tbl_nn_puk -- => WARNING -- @@ -194,24 +194,24 @@ CREATE UNIQUE INDEX tbl_nn_puk_pcol1_idx ON tbl_nn_puk(col1) WHERE col1 < 10; CREATE TABLE issue3_1 (col1 int NOT NULL, col2 text NOT NULL); CREATE UNIQUE INDEX issue3_1_idx ON issue3_1 (col1, col2 DESC); SELECT repack.get_order_by('issue3_1_idx'::regclass::oid, 'issue3_1'::regclass::oid); -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_1 +\! pg_repack --dbname=contrib_regression --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); -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_2 +\! pg_repack --dbname=contrib_regression --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); -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_3 +\! pg_repack --dbname=contrib_regression --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); -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_4 +\! pg_repack --dbname=contrib_regression --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); -\! pg_repack --dbname=contrib_regression --no-order --table=issue3_5 +\! pg_repack --dbname=contrib_regression --table=issue3_5 diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 9692081..e03dea8 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -253,13 +253,13 @@ Environment Examples -------- -Execute the following command to perform an online CLUSTER of all tables in -database ``test``:: +Perform an online CLUSTER of all the clustered tables in the database +``test``, and perform an online VACUUM FULL of all the non-clustered tables:: $ pg_repack test -Execute the following command to perform an online VACUUM FULL of table -``foo`` in database ``test``:: +Perform an online VACUUM FULL on the table ``foo`` in the database ``test`` +(an eventual cluster index is ignored):: $ pg_repack --no-order --table foo -d test @@ -319,7 +319,7 @@ ERROR: relation "table" has no cluster key Define a CLUSTER KEY on the table, via ALTER TABLE CLUSTER ON, or use one of the --no-order or --order-by modes. -pg_repack: query failed: ERROR: column "col" does not exist +ERROR: query failed: ERROR: column "col" does not exist The target table doesn't have columns specified by ``--order-by`` option. Specify existing columns. @@ -404,6 +404,8 @@ Releases * pg_repack 1.2 * Added --jobs option for parallel operation. + * Don't require --no-order to perform a VACUUM FULL on non-clustered tables + (pg_repack issue #6). * More helpful error messages. * Bugfix: correctly handle key indexes with options such as DESC, NULL FIRST/LAST, COLLATE (pg_repack issue #3). From 7617e07f100173f4de062b10e5d1218ab7368de3 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 01:44:50 +0100 Subject: [PATCH 11/33] Options sorted in a slightly more rational order --no-order now is almost useless, but list it next to --order-by. --jobs only specifies how to do something, not what to do. On the same basis probably --no-analyze should be pushed further up. --- bin/pg_repack.c | 6 +++--- doc/pg_repack.rst | 38 ++++++++++++++++++++------------------ 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 3460379..d25d3d7 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -1452,10 +1452,10 @@ pgut_help(bool details) printf("Options:\n"); printf(" -a, --all repack all databases\n"); - printf(" -j --jobs Use this many parallel jobs for each table\n"); - printf(" -n, --no-order do vacuum full instead of cluster\n"); - printf(" -o, --order-by=COLUMNS order by columns instead of cluster keys\n"); printf(" -t, --table=TABLE repack specific table only\n"); + printf(" -o, --order-by=COLUMNS order by columns instead of cluster keys\n"); + printf(" -n, --no-order do vacuum full instead of cluster\n"); + printf(" -j --jobs Use this many parallel jobs for each table\n"); printf(" -T, --wait-timeout=SECS timeout to cancel other backends on conflict\n"); printf(" -Z, --no-analyze don't analyze at end\n"); } diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index e03dea8..4d2e38e 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -117,10 +117,10 @@ The following options can be specified in ``OPTIONS``. Options: -a, --all repack all databases - -j, --jobs Use this many parallel jobs for each table - -n, --no-order do vacuum full instead of cluster - -o, --order-by=COLUMNS order by columns instead of cluster keys -t, --table=TABLE repack specific table only + -o, --order-by=COLUMNS order by columns instead of cluster keys + -n, --no-order do vacuum full instead of cluster + -j, --jobs Use this many parallel jobs for each table -T, --wait-timeout=SECS timeout to cancel other backends on conflict -Z, --no-analyze don't analyze at end @@ -142,26 +142,27 @@ Generic options: Reorg Options ^^^^^^^^^^^^^ -Options to order rows. If not specified, pg_repack performs an online CLUSTER -using cluster indexes. Only one option can be specified. You may also specify -target tables or databases. - -``-j``, ``--jobs`` - Create the specified number of extra connections to PostgreSQL, and - use these extra connections to parallelize the rebuild of indexes - on each table. If your PostgreSQL server has extra cores and disk - I/O available, this can be a useful way to speed up pg_repack. - -``-n``, ``--no-order`` - Perform an online VACUUM FULL. - -``-o COLUMNS [,...]``, ``--order-by=COLUMNS [,...]`` - Perform an online CLUSTER ordered by the specified columns. +``-a``, ``--all`` + Attempt repack all the databases of the cluster. Databases where the + ``pg_repack`` extension is not installed will be skipped. ``-t TABLE``, ``--table=TABLE`` Reorganize the specified table only. By default, all eligible tables in the target databases are reorganized. +``-o COLUMNS [,...]``, ``--order-by=COLUMNS [,...]`` + Perform an online CLUSTER ordered by the specified columns. + +``-n``, ``--no-order`` + Perform an online VACUUM FULL. Since version 1.2 this is the default for + non-clustered tables. + +``-j``, ``--jobs`` + Create the specified number of extra connections to PostgreSQL, and + use these extra connections to parallelize the rebuild of indexes + on each table. If your PostgreSQL server has extra cores and disk + I/O available, this can be a useful way to speed up pg_repack. + ``-T SECS``, ``--wait-timeout=SECS`` pg_repack needs to take an exclusive lock at the end of the reorganization. This setting controls how many seconds pg_repack will @@ -175,6 +176,7 @@ target tables or databases. Disable ANALYZE after the reorganization. If not specified, run ANALYZE after the reorganization. + Connection Options ^^^^^^^^^^^^^^^^^^ From 0e748824299cb23fb432ee8717b0ab3c6404f4cb Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 09:17:35 +0100 Subject: [PATCH 12/33] Had to shorten the tablespace metavar In the rst docs two spaces are required between option and doc. --- bin/pg_repack.c | 4 ++-- doc/pg_repack.rst | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 6569bc3..759434a 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -1542,8 +1542,8 @@ pgut_help(bool details) printf("Options:\n"); printf(" -a, --all repack all databases\n"); printf(" -t, --table=TABLE repack specific table only\n"); - printf(" -s, --tablespace=TABLESPC move repacked tables to a new tablespace\n"); - printf(" -S, --moveidx move repacked indexes to TABLESPC too\n"); + printf(" -s, --tablespace=TBLSPC move repacked tables to a new tablespace\n"); + printf(" -S, --moveidx move repacked indexes to TBLSPC too\n"); printf(" -o, --order-by=COLUMNS order by columns instead of cluster keys\n"); printf(" -n, --no-order do vacuum full instead of cluster\n"); printf(" -j --jobs Use this many parallel jobs for each table\n"); diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index d066eb5..43b1a76 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -118,8 +118,8 @@ The following options can be specified in ``OPTIONS``. Options: -a, --all repack all databases -t, --table=TABLE repack specific table only - -s, --tablespace=TABLESPC move repacked tables to a new tablespace - -S, --moveidx move repacked indexes to TABLESPC too + -s, --tablespace=TBLSPC move repacked tables to a new tablespace + -S, --moveidx move repacked indexes to TBLSPC too -o, --order-by=COLUMNS order by columns instead of cluster keys -n, --no-order do vacuum full instead of cluster -j, --jobs Use this many parallel jobs for each table @@ -165,7 +165,7 @@ Reorg Options on each table. If your PostgreSQL server has extra cores and disk I/O available, this can be a useful way to speed up pg_repack. -``-s TABLESPC``, ``--tablespace=TABLESPC`` +``-s TBLSPC``, ``--tablespace=TBLSPC`` Move the repacked tables to the specified tablespace: essentially an online version of ``ALTER TABLE ... SET TABLESPACE``. The tables indexes are left on the original tablespace unless ``--moveidx`` is specified too. From 700a1a6c48dfc382e3b55a314eb7b13fe8931cd9 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Fri, 23 Nov 2012 00:55:53 +0000 Subject: [PATCH 13/33] 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 14/33] 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 15/33] 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 16/33] 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 17/33] 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); From 3d0b02c654e8726ee13f6ec7e28f7f1f951cac15 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 18:56:50 +0100 Subject: [PATCH 18/33] Try to repack all tables with pkey, not only the ones with ckey This is consistent with --no-order not been required anymore. --- bin/expected/repack.out | 9 +++++++-- bin/pg_repack.c | 2 -- bin/sql/repack.sql | 2 +- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/bin/expected/repack.out b/bin/expected/repack.out index 3beabdb..76518f6 100644 --- a/bin/expected/repack.out +++ b/bin/expected/repack.out @@ -114,15 +114,20 @@ SELECT * FROM tbl_with_dropped_toast; -- -- do repack -- +\! pg_repack --dbname=contrib_regression --table=tbl_cluster +INFO: repacking table "tbl_cluster" \! 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_only_pkey" +INFO: repacking table "tbl_gistkey" 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" +INFO: repacking table "tbl_badindex" +WARNING: skipping invalid index: CREATE UNIQUE INDEX idx_badindex_n ON tbl_badindex USING btree (n) +INFO: repacking table "tbl_idxopts" -- -- after -- diff --git a/bin/pg_repack.c b/bin/pg_repack.c index b515924..1d84d9f 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -521,8 +521,6 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) else { appendStringInfoString(&sql, "pkid IS NOT NULL"); - if (!orderby) - appendStringInfoString(&sql, " AND ckid IS NOT NULL"); } /* double check the parameters array is sane */ diff --git a/bin/sql/repack.sql b/bin/sql/repack.sql index b3a4a0e..c84a2a2 100644 --- a/bin/sql/repack.sql +++ b/bin/sql/repack.sql @@ -120,9 +120,9 @@ SELECT * FROM tbl_with_dropped_toast; -- do repack -- +\! pg_repack --dbname=contrib_regression --table=tbl_cluster \! pg_repack --dbname=contrib_regression --table=tbl_badindex \! pg_repack --dbname=contrib_regression -\! pg_repack --dbname=contrib_regression --table=tbl_cluster -- -- after From db40e71abc10daa4350e767eb6aab2242758bab4 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 22:07:36 +0100 Subject: [PATCH 19/33] Fixed query for PG 8.4 Posgtres cannot find a cast rule. --- bin/pg_repack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 1d84d9f..d2f6575 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -93,7 +93,7 @@ const char *PROGRAM_VERSION = "unknown"; "SELECT repack.array_accum(virtualtransaction) FROM pg_locks" \ " WHERE locktype = 'virtualxid' AND pid NOT IN (pg_backend_pid(), $1)" \ " AND (virtualxid, virtualtransaction) <> ('1/1', '-1/0') " \ - " AND ($2 IS NOT NULL)" + " AND ($2::text IS NOT NULL)" #define SQL_XID_SNAPSHOT \ (PQserverVersion(connection) >= 90200 ? SQL_XID_SNAPSHOT_90200 : \ From 22762fce2869db80944dfc3c36839c82e7c73119 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 22:07:54 +0100 Subject: [PATCH 20/33] Fixed query for PG 8.3 The AS keyword was required. --- lib/pg_repack.sql.in | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index a223976..d4c333a 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -180,7 +180,7 @@ CREATE VIEW repack.tables AS repack.get_create_trigger(R.oid, PK.indexrelid) AS create_trigger, repack.get_enable_trigger(R.oid) as enable_trigger, 'CREATE TABLE repack.table_' || R.oid || ' WITH (' || array_to_string(array_append(R.reloptions, 'oids=' || CASE WHEN R.relhasoids THEN 'true' ELSE 'false' END), ',') || ') TABLESPACE ' AS create_table_1, - coalesce(quote_ident(S.spcname), 'pg_default') tablespace_orig, + coalesce(quote_ident(S.spcname), 'pg_default') as tablespace_orig, ' AS SELECT ' || repack.get_columns_for_create_as(R.oid) || ' FROM ONLY ' || repack.oid2text(R.oid) AS create_table_2, repack.get_drop_columns(R.oid, 'repack.table_' || R.oid) AS drop_columns, 'DELETE FROM repack.log_' || R.oid AS delete_log, From c314cbda75848941fde8c3030e0f829c96276aa9 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 18 Apr 2013 00:29:23 +0100 Subject: [PATCH 21/33] Check PostgreSQL version number as number in the makefile Not entirely happy about the solution but I like the uniform tests. --- Makefile | 12 +++---- bin/Makefile | 38 ++++++++-------------- bin/expected/init-extension.out | 3 ++ bin/expected/{init.out => init-legacy.out} | 0 bin/sql/init-extension.sql | 2 -- lib/Makefile | 11 ++++--- 6 files changed, 29 insertions(+), 37 deletions(-) create mode 100644 bin/expected/init-extension.out rename bin/expected/{init.out => init-legacy.out} (100%) diff --git a/Makefile b/Makefile index 1e43714..d74c55a 100644 --- a/Makefile +++ b/Makefile @@ -6,18 +6,18 @@ # Portions Copyright (c) 2012, The Reorg Development Team # -USE_PGXS = 1 -PG_CONFIG = pg_config -PGXS := $(shell $(PG_CONFIG) --pgxs) -include $(PGXS) +PG_CONFIG ?= pg_config SUBDIRS = bin lib # Pull out the version number from pg_config -VERSION = $(shell $(PG_CONFIG) --version | awk '{print $$2}') +VERSION := $(shell $(PG_CONFIG) --version | awk '{print $$2}') + +# version as a number, e.g. 9.1.4 -> 90104 +INTVERSION := $(shell echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+)\.?([0-9]+)?(.*)/(\1*100+\2)*100+0\3/' | bc) # We support PostgreSQL 8.3 and later. -ifneq ($(shell echo $(VERSION) | grep -E "^7\.|^8\.[012]"),) +ifeq ($(shell echo $$(($(INTVERSION) < 80300))),1) $(error pg_repack requires PostgreSQL 8.3 or later. This is $(VERSION)) endif diff --git a/bin/Makefile b/bin/Makefile index 24ae1cd..d871e30 100644 --- a/bin/Makefile +++ b/bin/Makefile @@ -5,12 +5,24 @@ # Portions Copyright (c) 2011, Itagaki Takahiro # Portions Copyright (c) 2012, The Reorg Development Team # + +PG_CONFIG ?= pg_config + +# version as a number, e.g. 9.1.4 -> 90104 +VERSION := $(shell $(PG_CONFIG) --version | awk '{print $$2}') +INTVERSION := $(shell echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+)\.?([0-9]+)?.*/(\1*100+\2)*100+0\3/' | bc) + SRCS = pg_repack.c pgut/pgut.c pgut/pgut-fe.c OBJS = $(SRCS:.c=.o) PROGRAM = pg_repack -REGRESS = init repack tablespace -EXTRA_CLEAN = sql/init-$(MAJORVERSION).sql sql/init.sql +ifeq ($(shell echo $$(($(INTVERSION) >= 90100))),1) +REGRESS = init-extension +else +REGRESS = init-legacy +endif + +REGRESS += repack tablespace # The version number of the program. It should be the same of the library. REPACK_VERSION = $(shell grep '"version":' ../META.json | head -1 \ @@ -33,25 +45,3 @@ include $(PGXS) LIBS := $(filter-out -lxml2, $(LIBS)) LIBS := $(filter-out -lxslt, $(LIBS)) -ifndef MAJORVERSION -MAJORVERSION := $(basename $(VERSION)) -endif - -sql/init.sql: sql/init-$(MAJORVERSION).sql - cp sql/init-$(MAJORVERSION).sql sql/init.sql -expected/init.out: expected/init-$(MAJORVERSION).out - cp expected/init-$(MAJORVERSION).out expected/init.out -sql/init-8.3.sql: - cp sql/init-legacy.sql sql/init-8.3.sql -sql/init-8.4.sql: - cp sql/init-legacy.sql sql/init-8.4.sql -sql/init-9.0.sql: - cp sql/init-legacy.sql sql/init-9.0.sql -sql/init-9.1.sql: - cp sql/init-extension.sql sql/init-9.1.sql -sql/init-9.2.sql: - cp sql/init-extension.sql sql/init-9.2.sql -sql/init-9.3.sql: - cp sql/init-extension.sql sql/init-9.3.sql - -installcheck: sql/init.sql diff --git a/bin/expected/init-extension.out b/bin/expected/init-extension.out new file mode 100644 index 0000000..9f2e171 --- /dev/null +++ b/bin/expected/init-extension.out @@ -0,0 +1,3 @@ +SET client_min_messages = warning; +CREATE EXTENSION pg_repack; +RESET client_min_messages; diff --git a/bin/expected/init.out b/bin/expected/init-legacy.out similarity index 100% rename from bin/expected/init.out rename to bin/expected/init-legacy.out diff --git a/bin/sql/init-extension.sql b/bin/sql/init-extension.sql index 9da3176..9f2e171 100644 --- a/bin/sql/init-extension.sql +++ b/bin/sql/init-extension.sql @@ -1,5 +1,3 @@ SET client_min_messages = warning; -\set ECHO none CREATE EXTENSION pg_repack; -\set ECHO all RESET client_min_messages; diff --git a/lib/Makefile b/lib/Makefile index ca38f3a..8698163 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -6,7 +6,11 @@ # Portions Copyright (c) 2012, The Reorg Development Team # -PG_CONFIG = pg_config +PG_CONFIG ?= pg_config + +# version as a number, e.g. 9.1.4 -> 90104 +VERSION := $(shell $(PG_CONFIG) --version | awk '{print $$2}') +INTVERSION := $(shell echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+)\.?([0-9]+)?.*/(\1*100+\2)*100+0\3/' | bc) EXTENSION = pg_repack MODULE_big = $(EXTENSION) @@ -20,10 +24,7 @@ REPACK_VERSION = $(shell grep '"version":' ../META.json | head -1 \ PG_CPPFLAGS = -DREPACK_VERSION=$(REPACK_VERSION) # Support CREATE EXTENSION for PG >= 9.1 and a simple sql script for PG < 9.1 -HAVE_EXTENSION = $(shell $(PG_CONFIG) --version \ - | grep -qE " 8\.| 9\.0" && echo no || echo yes) - -ifeq ($(HAVE_EXTENSION),yes) +ifeq ($(shell echo $$(($(INTVERSION) >= 90100))),1) DATA_built = pg_repack--$(REPACK_VERSION).sql pg_repack.control else DATA_built = pg_repack.sql From dd06f2593038f37880f8a767cefc5dc7e0a74ed0 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 18 Apr 2013 00:34:44 +0100 Subject: [PATCH 22/33] Tests on COLLATE only run on PG versions supporting it --- bin/Makefile | 11 +++++++++ bin/expected/issue3.out | 53 +++++++++++++++++++++++++++++++++++++++++ bin/expected/repack.out | 53 ----------------------------------------- bin/sql/issue3.sql | 27 +++++++++++++++++++++ bin/sql/repack.sql | 28 ---------------------- 5 files changed, 91 insertions(+), 81 deletions(-) create mode 100644 bin/expected/issue3.out create mode 100644 bin/sql/issue3.sql diff --git a/bin/Makefile b/bin/Makefile index d871e30..04f38f2 100644 --- a/bin/Makefile +++ b/bin/Makefile @@ -16,6 +16,11 @@ SRCS = pg_repack.c pgut/pgut.c pgut/pgut-fe.c OBJS = $(SRCS:.c=.o) PROGRAM = pg_repack + +# +# Test suite +# + ifeq ($(shell echo $$(($(INTVERSION) >= 90100))),1) REGRESS = init-extension else @@ -24,6 +29,12 @@ endif REGRESS += repack tablespace +# This test depends on collate, not supported before 9.0 +ifeq ($(shell echo $$(($(INTVERSION) >= 90000))),1) +REGRESS += issue3 +endif + + # The version number of the program. It should be the same of the library. REPACK_VERSION = $(shell grep '"version":' ../META.json | head -1 \ | sed -e 's/[ ]*"version":[ ]*"\(.*\)",/\1/') diff --git a/bin/expected/issue3.out b/bin/expected/issue3.out new file mode 100644 index 0000000..ee3e9a7 --- /dev/null +++ b/bin/expected/issue3.out @@ -0,0 +1,53 @@ +-- +-- pg_repack issue #3 +-- +CREATE TABLE issue3_1 (col1 int NOT NULL, col2 text NOT NULL); +CREATE UNIQUE INDEX issue3_1_idx ON issue3_1 (col1, col2 DESC); +SELECT repack.get_order_by('issue3_1_idx'::regclass::oid, 'issue3_1'::regclass::oid); + get_order_by +----------------- + col1, col2 DESC +(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); + get_order_by +--------------------------- + col1 DESC, col2 USING ~<~ +(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); + get_order_by +---------------------- + col1 DESC, col2 DESC +(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); + get_order_by +-------------------------------------------------- + col1 NULLS FIRST, col2 DESC USING ~<~ NULLS LAST +(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); + get_order_by +-------------------------------------- + col1 DESC, col2 COLLATE "POSIX" DESC +(1 row) + +\! pg_repack --dbname=contrib_regression --table=issue3_5 +INFO: repacking table "issue3_5" diff --git a/bin/expected/repack.out b/bin/expected/repack.out index 76518f6..9bcc4b4 100644 --- a/bin/expected/repack.out +++ b/bin/expected/repack.out @@ -326,56 +326,3 @@ INFO: repacking table "tbl_pk_uk" \! pg_repack --dbname=contrib_regression --table=tbl_nn_puk WARNING: relation "tbl_nn_puk" must have a primary key or not-null unique keys -- => WARNING --- --- pg_repack issue #3 --- -CREATE TABLE issue3_1 (col1 int NOT NULL, col2 text NOT NULL); -CREATE UNIQUE INDEX issue3_1_idx ON issue3_1 (col1, col2 DESC); -SELECT repack.get_order_by('issue3_1_idx'::regclass::oid, 'issue3_1'::regclass::oid); - get_order_by ------------------ - col1, col2 DESC -(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); - get_order_by ---------------------------- - col1 DESC, col2 USING ~<~ -(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); - get_order_by ----------------------- - col1 DESC, col2 DESC -(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); - get_order_by --------------------------------------------------- - col1 NULLS FIRST, col2 DESC USING ~<~ NULLS LAST -(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); - get_order_by --------------------------------------- - col1 DESC, col2 COLLATE "POSIX" DESC -(1 row) - -\! pg_repack --dbname=contrib_regression --table=issue3_5 -INFO: repacking table "issue3_5" diff --git a/bin/sql/issue3.sql b/bin/sql/issue3.sql new file mode 100644 index 0000000..9065488 --- /dev/null +++ b/bin/sql/issue3.sql @@ -0,0 +1,27 @@ +-- +-- pg_repack issue #3 +-- +CREATE TABLE issue3_1 (col1 int NOT NULL, col2 text NOT NULL); +CREATE UNIQUE INDEX issue3_1_idx ON issue3_1 (col1, col2 DESC); +SELECT repack.get_order_by('issue3_1_idx'::regclass::oid, 'issue3_1'::regclass::oid); +\! pg_repack --dbname=contrib_regression --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); +\! pg_repack --dbname=contrib_regression --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); +\! pg_repack --dbname=contrib_regression --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); +\! pg_repack --dbname=contrib_regression --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); +\! pg_repack --dbname=contrib_regression --table=issue3_5 diff --git a/bin/sql/repack.sql b/bin/sql/repack.sql index c84a2a2..3a6f82f 100644 --- a/bin/sql/repack.sql +++ b/bin/sql/repack.sql @@ -187,31 +187,3 @@ CREATE UNIQUE INDEX tbl_nn_puk_pcol1_idx ON tbl_nn_puk(col1) WHERE col1 < 10; -- => OK \! pg_repack --dbname=contrib_regression --table=tbl_nn_puk -- => WARNING - --- --- pg_repack issue #3 --- -CREATE TABLE issue3_1 (col1 int NOT NULL, col2 text NOT NULL); -CREATE UNIQUE INDEX issue3_1_idx ON issue3_1 (col1, col2 DESC); -SELECT repack.get_order_by('issue3_1_idx'::regclass::oid, 'issue3_1'::regclass::oid); -\! pg_repack --dbname=contrib_regression --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); -\! pg_repack --dbname=contrib_regression --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); -\! pg_repack --dbname=contrib_regression --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); -\! pg_repack --dbname=contrib_regression --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); -\! pg_repack --dbname=contrib_regression --table=issue3_5 From 14c4d4653e48b97fddd6937c8a383db25a4a9b96 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 18 Apr 2013 01:10:22 +0100 Subject: [PATCH 23/33] Make the version number arith without using bc Not as available as I thought. Can't use the 0 prefix to make the 3rd number optional as $(()) parses is as octal, so only use the first 2 numbers. Also fixed collate test: not available on PG 9.0. --- Makefile | 13 ++++++++----- bin/Makefile | 10 +++++----- lib/Makefile | 6 +++--- 3 files changed, 16 insertions(+), 13 deletions(-) diff --git a/Makefile b/Makefile index d74c55a..a10744e 100644 --- a/Makefile +++ b/Makefile @@ -8,20 +8,23 @@ PG_CONFIG ?= pg_config -SUBDIRS = bin lib - # Pull out the version number from pg_config VERSION := $(shell $(PG_CONFIG) --version | awk '{print $$2}') +ifeq ("$(VERSION)","") +$(error pg_config not found) +endif -# version as a number, e.g. 9.1.4 -> 90104 -INTVERSION := $(shell echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+)\.?([0-9]+)?(.*)/(\1*100+\2)*100+0\3/' | bc) +# version as a number, e.g. 9.1.4 -> 901 +INTVERSION := $(shell echo $$(($$(echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+).*/\1*100+\2/')))) # We support PostgreSQL 8.3 and later. -ifeq ($(shell echo $$(($(INTVERSION) < 80300))),1) +ifeq ($(shell echo $$(($(INTVERSION) < 803))),1) $(error pg_repack requires PostgreSQL 8.3 or later. This is $(VERSION)) endif +SUBDIRS = bin lib + all install installdirs uninstall distprep clean distclean maintainer-clean debug: @for dir in $(SUBDIRS); do \ $(MAKE) -C $$dir $@ || exit; \ diff --git a/bin/Makefile b/bin/Makefile index 04f38f2..1193e5f 100644 --- a/bin/Makefile +++ b/bin/Makefile @@ -8,9 +8,9 @@ PG_CONFIG ?= pg_config -# version as a number, e.g. 9.1.4 -> 90104 +# version as a number, e.g. 9.1.4 -> 901 VERSION := $(shell $(PG_CONFIG) --version | awk '{print $$2}') -INTVERSION := $(shell echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+)\.?([0-9]+)?.*/(\1*100+\2)*100+0\3/' | bc) +INTVERSION := $(shell echo $$(($$(echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+).*/\1*100+\2/')))) SRCS = pg_repack.c pgut/pgut.c pgut/pgut-fe.c OBJS = $(SRCS:.c=.o) @@ -21,7 +21,7 @@ PROGRAM = pg_repack # Test suite # -ifeq ($(shell echo $$(($(INTVERSION) >= 90100))),1) +ifeq ($(shell echo $$(($(INTVERSION) >= 901))),1) REGRESS = init-extension else REGRESS = init-legacy @@ -29,8 +29,8 @@ endif REGRESS += repack tablespace -# This test depends on collate, not supported before 9.0 -ifeq ($(shell echo $$(($(INTVERSION) >= 90000))),1) +# This test depends on collate, not supported before 9.1 +ifeq ($(shell echo $$(($(INTVERSION) >= 901))),1) REGRESS += issue3 endif diff --git a/lib/Makefile b/lib/Makefile index 8698163..1035742 100644 --- a/lib/Makefile +++ b/lib/Makefile @@ -8,9 +8,9 @@ PG_CONFIG ?= pg_config -# version as a number, e.g. 9.1.4 -> 90104 +# version as a number, e.g. 9.1.4 -> 901 VERSION := $(shell $(PG_CONFIG) --version | awk '{print $$2}') -INTVERSION := $(shell echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+)\.?([0-9]+)?.*/(\1*100+\2)*100+0\3/' | bc) +INTVERSION := $(shell echo $$(($$(echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+).*/\1*100+\2/')))) EXTENSION = pg_repack MODULE_big = $(EXTENSION) @@ -24,7 +24,7 @@ REPACK_VERSION = $(shell grep '"version":' ../META.json | head -1 \ PG_CPPFLAGS = -DREPACK_VERSION=$(REPACK_VERSION) # Support CREATE EXTENSION for PG >= 9.1 and a simple sql script for PG < 9.1 -ifeq ($(shell echo $$(($(INTVERSION) >= 90100))),1) +ifeq ($(shell echo $$(($(INTVERSION) >= 901))),1) DATA_built = pg_repack--$(REPACK_VERSION).sql pg_repack.control else DATA_built = pg_repack.sql From 477fba884c777ca7e8bd65eed574de882e524db1 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Wed, 17 Apr 2013 19:10:24 +0100 Subject: [PATCH 24/33] Ignore AFTER triggers sorting after z_repack_trigger They are not a problem, and they fore after the BEFORE trigger anyway. To be checked against more PostgreSQL versions. --- bin/expected/repack.out | 26 ++++++++++++++++++++++++++ bin/sql/repack.sql | 19 +++++++++++++++++++ lib/pg_repack.sql.in | 1 + 3 files changed, 46 insertions(+) diff --git a/bin/expected/repack.out b/bin/expected/repack.out index 76518f6..26c5797 100644 --- a/bin/expected/repack.out +++ b/bin/expected/repack.out @@ -379,3 +379,29 @@ SELECT repack.get_order_by('issue3_5_idx'::regclass::oid, 'issue3_5'::regclass:: \! pg_repack --dbname=contrib_regression --table=issue3_5 INFO: repacking table "issue3_5" +-- +-- Triggers handling +-- +CREATE FUNCTION trgtest() RETURNS trigger AS +$$BEGIN RETURN NEW; END$$ +LANGUAGE plpgsql; +CREATE TABLE trg1 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg1 +INFO: repacking table "trg1" +CREATE TABLE trg2 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg2 +INFO: repacking table "trg2" +WARNING: the table "trg2" has already a trigger called "z_repack_trigger" +DETAIL: The trigger was probably installed during a previous attempt to run pg_repack on the table which was interrupted and for some reason failed to clean up the temporary objects. Please drop the trigger or drop and recreate the pg_repack extension altogether to remove all the temporary objects left over. +CREATE TABLE trg3 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg3 +INFO: repacking table "trg3" +WARNING: trigger "z_repack_trigges" conflicting on table "trg3" +DETAIL: The trigger "z_repack_trigger" must be the last of the BEFORE triggers to fire on the table (triggers fire in alphabetical order). Please rename the trigger so that it sorts before "z_repack_trigger": you can use "ALTER TRIGGER z_repack_trigges ON trg3 RENAME TO newname". +CREATE TABLE trg4 (id integer PRIMARY KEY); +CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg4 +INFO: repacking table "trg4" diff --git a/bin/sql/repack.sql b/bin/sql/repack.sql index c84a2a2..f705c2c 100644 --- a/bin/sql/repack.sql +++ b/bin/sql/repack.sql @@ -215,3 +215,22 @@ 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); \! pg_repack --dbname=contrib_regression --table=issue3_5 + +-- +-- Triggers handling +-- +CREATE FUNCTION trgtest() RETURNS trigger AS +$$BEGIN RETURN NEW; END$$ +LANGUAGE plpgsql; +CREATE TABLE trg1 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg1 +CREATE TABLE trg2 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg2 +CREATE TABLE trg3 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg3 +CREATE TABLE trg4 (id integer PRIMARY KEY); +CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg4 diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index d4c333a..d002872 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -219,6 +219,7 @@ CREATE FUNCTION repack.conflicted_triggers(oid) RETURNS SETOF name AS $$ SELECT tgname FROM pg_trigger WHERE tgrelid = $1 AND tgname >= 'z_repack_trigger' + AND (tgtype & 2) = 2 -- BEFORE trigger ORDER BY tgname; $$ LANGUAGE sql STABLE STRICT; From a2138b6d7d3a42cc7542c00d12fc03f29b23494e Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 18 Apr 2013 01:22:24 +0100 Subject: [PATCH 25/33] Ignore AFTER triggers sorting after z_repack_trigger They are not a problem, and they fore after the BEFORE trigger anyway. To be checked against more PostgreSQL versions. --- bin/expected/repack.out | 26 ++++++++++++++++++++++++++ bin/sql/repack.sql | 19 +++++++++++++++++++ lib/pg_repack.sql.in | 1 + 3 files changed, 46 insertions(+) diff --git a/bin/expected/repack.out b/bin/expected/repack.out index 9bcc4b4..1536b02 100644 --- a/bin/expected/repack.out +++ b/bin/expected/repack.out @@ -326,3 +326,29 @@ INFO: repacking table "tbl_pk_uk" \! pg_repack --dbname=contrib_regression --table=tbl_nn_puk WARNING: relation "tbl_nn_puk" must have a primary key or not-null unique keys -- => WARNING +-- +-- Triggers handling +-- +CREATE FUNCTION trgtest() RETURNS trigger AS +$$BEGIN RETURN NEW; END$$ +LANGUAGE plpgsql; +CREATE TABLE trg1 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg1 +INFO: repacking table "trg1" +CREATE TABLE trg2 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg2 +INFO: repacking table "trg2" +WARNING: the table "trg2" has already a trigger called "z_repack_trigger" +DETAIL: The trigger was probably installed during a previous attempt to run pg_repack on the table which was interrupted and for some reason failed to clean up the temporary objects. Please drop the trigger or drop and recreate the pg_repack extension altogether to remove all the temporary objects left over. +CREATE TABLE trg3 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg3 +INFO: repacking table "trg3" +WARNING: trigger "z_repack_trigges" conflicting on table "trg3" +DETAIL: The trigger "z_repack_trigger" must be the last of the BEFORE triggers to fire on the table (triggers fire in alphabetical order). Please rename the trigger so that it sorts before "z_repack_trigger": you can use "ALTER TRIGGER z_repack_trigges ON trg3 RENAME TO newname". +CREATE TABLE trg4 (id integer PRIMARY KEY); +CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg4 +INFO: repacking table "trg4" diff --git a/bin/sql/repack.sql b/bin/sql/repack.sql index 3a6f82f..e460c2d 100644 --- a/bin/sql/repack.sql +++ b/bin/sql/repack.sql @@ -187,3 +187,22 @@ CREATE UNIQUE INDEX tbl_nn_puk_pcol1_idx ON tbl_nn_puk(col1) WHERE col1 < 10; -- => OK \! pg_repack --dbname=contrib_regression --table=tbl_nn_puk -- => WARNING + +-- +-- Triggers handling +-- +CREATE FUNCTION trgtest() RETURNS trigger AS +$$BEGIN RETURN NEW; END$$ +LANGUAGE plpgsql; +CREATE TABLE trg1 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg1 +CREATE TABLE trg2 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg2 +CREATE TABLE trg3 (id integer PRIMARY KEY); +CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg3 +CREATE TABLE trg4 (id integer PRIMARY KEY); +CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest(); +\! pg_repack --dbname=contrib_regression --table=trg4 diff --git a/lib/pg_repack.sql.in b/lib/pg_repack.sql.in index d4c333a..d002872 100644 --- a/lib/pg_repack.sql.in +++ b/lib/pg_repack.sql.in @@ -219,6 +219,7 @@ CREATE FUNCTION repack.conflicted_triggers(oid) RETURNS SETOF name AS $$ SELECT tgname FROM pg_trigger WHERE tgrelid = $1 AND tgname >= 'z_repack_trigger' + AND (tgtype & 2) = 2 -- BEFORE trigger ORDER BY tgname; $$ LANGUAGE sql STABLE STRICT; From 54ba3c19cd3c995a703e886267e332929c591759 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 18 Apr 2013 01:32:05 +0100 Subject: [PATCH 26/33] Install plpgsql on test databases 8.3 and 8.4 Required by triggers tests --- bin/Makefile | 9 +++++++-- bin/expected/plpgsql.out | 1 + bin/sql/plpgsql.sql | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) create mode 100644 bin/expected/plpgsql.out create mode 100644 bin/sql/plpgsql.sql diff --git a/bin/Makefile b/bin/Makefile index 1193e5f..c7226c8 100644 --- a/bin/Makefile +++ b/bin/Makefile @@ -22,9 +22,14 @@ PROGRAM = pg_repack # ifeq ($(shell echo $$(($(INTVERSION) >= 901))),1) -REGRESS = init-extension +REGRESS := init-extension else -REGRESS = init-legacy +REGRESS := init-legacy +endif + +# plpgsql not available by default on pg < 9.0 +ifeq ($(shell echo $$(($(INTVERSION) < 900))),1) +REGRESS += plpgsql endif REGRESS += repack tablespace diff --git a/bin/expected/plpgsql.out b/bin/expected/plpgsql.out new file mode 100644 index 0000000..bc028fb --- /dev/null +++ b/bin/expected/plpgsql.out @@ -0,0 +1 @@ +CREATE LANGUAGE plpgsql; diff --git a/bin/sql/plpgsql.sql b/bin/sql/plpgsql.sql new file mode 100644 index 0000000..bc028fb --- /dev/null +++ b/bin/sql/plpgsql.sql @@ -0,0 +1 @@ +CREATE LANGUAGE plpgsql; From fb07fad34a73179278df6e79108ce09548c5c7b5 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 18 Apr 2013 02:29:51 +0100 Subject: [PATCH 27/33] Make sure to close the transactions after repack_one_table If we exit early (e.g. checking the triggers) repack_cleanup() is not called but we must close the transactions anyway. --- bin/pg_repack.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index d2f6575..a656e1c 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -1253,6 +1253,10 @@ cleanup: if (vxid) free(vxid); + /* Rollback current transactions */ + pgut_rollback(connection); + pgut_rollback(conn2); + /* XXX: distinguish between fatal and non-fatal errors via the first * arg to repack_cleanup(). */ @@ -1491,10 +1495,6 @@ repack_cleanup(bool fatal, const repack_table *table) char buffer[12]; const char *params[1]; - /* Rollback current transactions */ - pgut_rollback(connection); - pgut_rollback(conn2); - /* Try reconnection if not available. */ if (PQstatus(connection) != CONNECTION_OK || PQstatus(conn2) != CONNECTION_OK) From 52e77613433558190de58d95c0bf6bb2dea3dbc1 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 18 Apr 2013 02:49:56 +0100 Subject: [PATCH 28/33] Fixed doc formatting and dropped stale diagnostics entries --- doc/pg_repack.rst | 30 ++++++++++-------------------- 1 file changed, 10 insertions(+), 20 deletions(-) diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 43b1a76..a85e03c 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -119,7 +119,7 @@ Options: -a, --all repack all databases -t, --table=TABLE repack specific table only -s, --tablespace=TBLSPC move repacked tables to a new tablespace - -S, --moveidx move repacked indexes to TBLSPC too + -S, --moveidx move repacked indexes to *TBLSPC* too -o, --order-by=COLUMNS order by columns instead of cluster keys -n, --no-order do vacuum full instead of cluster -j, --jobs Use this many parallel jobs for each table @@ -293,13 +293,13 @@ database where the error occured and then load .. class:: diag -pg_repack: repack database "template1" ... skipped: pg_repack is not installed in the database - pg_repack is not installed in the database when ``--all`` option is +INFO: database "db" skipped: pg_repack VER is not installed in the database + pg_repack is not installed in the database when the ``--all`` option is specified. Create the pg_repack extension in the database. -ERROR: pg_repack is not installed +ERROR: pg_repack VER is not installed in the database pg_repack is not installed in the database specified by ``--dbname``. Create the pg_repack extension in the database. @@ -326,22 +326,11 @@ ERROR: relation "table" must have a primary key or not-null unique keys Define a PRIMARY KEY or a UNIQUE constraint on the table. -ERROR: relation "table" has no cluster key - The target table doesn't have CLUSTER KEY. - - Define a CLUSTER KEY on the table, via ALTER TABLE CLUSTER ON, or use - one of the --no-order or --order-by modes. - ERROR: query failed: ERROR: column "col" does not exist The target table doesn't have columns specified by ``--order-by`` option. Specify existing columns. -ERROR: permission denied for schema repack - Permission error. - - pg_repack must be executed by a superuser. - WARNING: the table "tbl" has already a trigger called z_repack_trigger The trigger was probably installed during a previous attempt to run pg_repack on the table which was interrupted and for some reason failed @@ -416,13 +405,14 @@ Releases * pg_repack 1.2 - * Added --tablespace and --moveidx options to perform online SET TABLESPACE. - * Added --jobs option for parallel operation. - * Don't require --no-order to perform a VACUUM FULL on non-clustered tables - (pg_repack issue #6). - * More helpful error messages. + * Added ``--tablespace`` and ``--moveidx`` options to perform online + SET TABLESPACE. + * Added ``--jobs`` option for parallel operation. + * Don't require ``--no-order`` to perform a VACUUM FULL on non-clustered + tables (pg_repack issue #6). * Bugfix: correctly handle key indexes with options such as DESC, NULL FIRST/LAST, COLLATE (pg_repack issue #3). + * More helpful program output and error messages. * pg_repack 1.1.8 From 109689586ef0fafaeaef422677c67f026082dc44 Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 22 Apr 2013 00:48:53 +0100 Subject: [PATCH 29/33] Fixed access to uninit'd mem in repack_indexdef If the tablespace is the last token in the indexdef, skip_ident returns a pointer *after* the term zero, so garbage may end up after the statement. --- lib/repack.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/repack.c b/lib/repack.c index 19cb089..125acce 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -662,12 +662,13 @@ repack_indexdef(PG_FUNCTION_ARGS) else { /* tablespace is to replace */ - char *tmp; + char *tmp, *limit; + limit = strchr(stmt.options, '\0'); tmp = skip_const(index, stmt.options, " TABLESPACE", NULL); appendStringInfoString(&str, stmt.options); appendStringInfo(&str, " %s", NameStr(*tablespace)); tmp = skip_ident(index, tmp); - if (*tmp) + if (tmp < limit) appendStringInfo(&str, " %s", tmp); } } From f886e0dba66d8fb46d3ee53be9b32370cf73817d Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Mon, 22 Apr 2013 11:16:28 +0100 Subject: [PATCH 30/33] Fixed --order-by broken by namespaces refactoring Also added a regression test for it. Bug and patch from Beena Emerson, thany you. --- bin/expected/tablespace.out | 3 +++ bin/pg_repack.c | 1 + bin/sql/tablespace.sql | 2 ++ 3 files changed, 6 insertions(+) diff --git a/bin/expected/tablespace.out b/bin/expected/tablespace.out index cf9a93e..7a7a769 100644 --- a/bin/expected/tablespace.out +++ b/bin/expected/tablespace.out @@ -78,3 +78,6 @@ ORDER BY relname; ERROR: cannot specify --moveidx (-S) without --tablespace (-s) \! pg_repack --dbname=contrib_regression --no-order --table=testts1 -S ERROR: cannot specify --moveidx (-S) without --tablespace (-s) +-- not broken with order +\! pg_repack --dbname=contrib_regression -o id --table=testts1 --tablespace pg_default --moveidx +INFO: repacking table "testts1" diff --git a/bin/pg_repack.c b/bin/pg_repack.c index a656e1c..ea3fd2a 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -617,6 +617,7 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) /* User specified ORDER BY */ appendStringInfoString(&sql, " ORDER BY "); appendStringInfoString(&sql, orderby); + table.create_table = sql.data; } repack_one_table(&table, orderby); diff --git a/bin/sql/tablespace.sql b/bin/sql/tablespace.sql index 73400e7..1f2c46a 100644 --- a/bin/sql/tablespace.sql +++ b/bin/sql/tablespace.sql @@ -53,3 +53,5 @@ ORDER BY relname; \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --moveidx \! pg_repack --dbname=contrib_regression --no-order --table=testts1 -S +-- not broken with order +\! pg_repack --dbname=contrib_regression -o id --table=testts1 --tablespace pg_default --moveidx From cc2abf4b25360e4ea4109ec79cfe38c9ddf10a1b Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 2 May 2013 23:32:30 +0100 Subject: [PATCH 31/33] Fixed tablespace assignment in index with WITH clause Reported by Beena Emerson. --- bin/expected/tablespace.out | 4 +++- bin/sql/tablespace.sql | 1 + lib/repack.c | 20 +++++++++++++++++++- 3 files changed, 23 insertions(+), 2 deletions(-) diff --git a/bin/expected/tablespace.out b/bin/expected/tablespace.out index 7a7a769..15f69c2 100644 --- a/bin/expected/tablespace.out +++ b/bin/expected/tablespace.out @@ -13,6 +13,7 @@ SELECT spcname FROM pg_tablespace WHERE spcname = 'testts'; -- If the query above failed you must create the 'testts' tablespace; CREATE TABLE testts1 (id serial primary key, data text); CREATE INDEX testts1_partial_idx on testts1 (id) where (id > 0); +CREATE INDEX testts1_with_idx on testts1 (id) with (fillfactor=80); INSERT INTO testts1 (data) values ('a'); INSERT INTO testts1 (data) values ('b'); INSERT INTO testts1 (data) values ('c'); @@ -71,7 +72,8 @@ ORDER BY relname; testts1 | testts testts1_partial_idx | testts testts1_pkey | testts -(3 rows) + testts1_with_idx | testts +(4 rows) -- can't specify --moveidx without --tablespace \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --moveidx diff --git a/bin/sql/tablespace.sql b/bin/sql/tablespace.sql index 1f2c46a..7fa2ce8 100644 --- a/bin/sql/tablespace.sql +++ b/bin/sql/tablespace.sql @@ -11,6 +11,7 @@ SELECT spcname FROM pg_tablespace WHERE spcname = 'testts'; CREATE TABLE testts1 (id serial primary key, data text); CREATE INDEX testts1_partial_idx on testts1 (id) where (id > 0); +CREATE INDEX testts1_with_idx on testts1 (id) with (fillfactor=80); INSERT INTO testts1 (data) values ('a'); INSERT INTO testts1 (data) values ('b'); INSERT INTO testts1 (data) values ('c'); diff --git a/lib/repack.c b/lib/repack.c index 125acce..34fdcf9 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -346,6 +346,24 @@ skip_const(Oid index, char *sql, const char *arg1, const char *arg2) return parse_error(index); } +static char * +skip_until_const(Oid index, char *sql, const char *what) +{ + char *pos; + + if ((pos = strstr(sql, what))) + { + size_t len; + + len = strlen(what); + pos[len] = '\0'; + return pos + len + 1; + } + + /* error */ + return parse_error(index); +} + static char * skip_ident(Oid index, char *sql) { @@ -664,7 +682,7 @@ repack_indexdef(PG_FUNCTION_ARGS) /* tablespace is to replace */ char *tmp, *limit; limit = strchr(stmt.options, '\0'); - tmp = skip_const(index, stmt.options, " TABLESPACE", NULL); + tmp = skip_until_const(index, stmt.options, " TABLESPACE"); appendStringInfoString(&str, stmt.options); appendStringInfo(&str, " %s", NameStr(*tablespace)); tmp = skip_ident(index, tmp); From 2b7e1b2f0c3f2ace82ffb9ca43f75fe0c3dbdb1e Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 4 May 2013 13:31:11 +0100 Subject: [PATCH 32/33] Ignore regression tests output --- .gitignore | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.gitignore b/.gitignore index 11ed7ca..7e82c4a 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,5 @@ # Global excludes across all subdirectories *.o *.so +bin/regression.diffs +bin/regression.out From a561c924f78671b83c857452a77b9a391b31299a Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Sat, 4 May 2013 23:59:01 +0100 Subject: [PATCH 33/33] Parse tablespace/where in parse_indexdef Makes injecting the target tablespace much easier and fixes interaction between tablespace and WITH/WHERE clauses. Added tests to check the correct indexes definition. --- bin/expected/tablespace.out | 25 +++++++++++ bin/sql/tablespace.sql | 13 ++++++ lib/repack.c | 85 ++++++++++++++++++++----------------- 3 files changed, 83 insertions(+), 40 deletions(-) diff --git a/bin/expected/tablespace.out b/bin/expected/tablespace.out index 15f69c2..2a51bc9 100644 --- a/bin/expected/tablespace.out +++ b/bin/expected/tablespace.out @@ -17,6 +17,31 @@ CREATE INDEX testts1_with_idx on testts1 (id) with (fillfactor=80); INSERT INTO testts1 (data) values ('a'); INSERT INTO testts1 (data) values ('b'); INSERT INTO testts1 (data) values ('c'); +-- check the indexes definitions +SELECT regexp_replace( + repack.repack_indexdef(indexrelid, 'testts1'::regclass, NULL), + '_[0-9]+', '_OID', 'g') +FROM pg_index i join pg_class c ON c.oid = indexrelid +WHERE indrelid = 'testts1'::regclass ORDER BY relname; + regexp_replace +---------------------------------------------------------------------------------- + CREATE INDEX index_OID ON repack.table_OID USING btree (id) WHERE (id > 0) + CREATE UNIQUE INDEX index_OID ON repack.table_OID USING btree (id) + CREATE INDEX index_OID ON repack.table_OID USING btree (id) WITH (fillfactor=80) +(3 rows) + +SELECT regexp_replace( + repack.repack_indexdef(indexrelid, 'testts1'::regclass, 'foo'), + '_[0-9]+', '_OID', 'g') +FROM pg_index i join pg_class c ON c.oid = indexrelid +WHERE indrelid = 'testts1'::regclass ORDER BY relname; + regexp_replace +------------------------------------------------------------------------------------------------- + CREATE INDEX index_OID ON repack.table_OID USING btree (id) TABLESPACE foo WHERE (id > 0) + CREATE UNIQUE INDEX index_OID ON repack.table_OID USING btree (id) TABLESPACE foo + CREATE INDEX index_OID ON repack.table_OID USING btree (id) WITH (fillfactor=80) TABLESPACE foo +(3 rows) + -- can move the tablespace from default \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts INFO: repacking table "testts1" diff --git a/bin/sql/tablespace.sql b/bin/sql/tablespace.sql index 7fa2ce8..60b9390 100644 --- a/bin/sql/tablespace.sql +++ b/bin/sql/tablespace.sql @@ -16,6 +16,19 @@ INSERT INTO testts1 (data) values ('a'); INSERT INTO testts1 (data) values ('b'); INSERT INTO testts1 (data) values ('c'); +-- check the indexes definitions +SELECT regexp_replace( + repack.repack_indexdef(indexrelid, 'testts1'::regclass, NULL), + '_[0-9]+', '_OID', 'g') +FROM pg_index i join pg_class c ON c.oid = indexrelid +WHERE indrelid = 'testts1'::regclass ORDER BY relname; + +SELECT regexp_replace( + repack.repack_indexdef(indexrelid, 'testts1'::regclass, 'foo'), + '_[0-9]+', '_OID', 'g') +FROM pg_index i join pg_class c ON c.oid = indexrelid +WHERE indrelid = 'testts1'::regclass ORDER BY relname; + -- can move the tablespace from default \! pg_repack --dbname=contrib_regression --no-order --table=testts1 --tablespace testts diff --git a/lib/repack.c b/lib/repack.c index 34fdcf9..a74de05 100644 --- a/lib/repack.c +++ b/lib/repack.c @@ -305,7 +305,9 @@ typedef struct IndexDef char *table; /* table name including schema */ char *type; /* btree, hash, gist or gin */ char *columns; /* column definition */ - char *options; /* options after columns. WITH, TABLESPACE and WHERE */ + char *options; /* options after columns, before TABLESPACE (e.g. COLLATE) */ + char *tablespace; /* tablespace if specified */ + char *where; /* WHERE content if specified */ } IndexDef; static char * @@ -349,14 +351,14 @@ skip_const(Oid index, char *sql, const char *arg1, const char *arg2) static char * skip_until_const(Oid index, char *sql, const char *what) { - char *pos; + char *pos; if ((pos = strstr(sql, what))) { - size_t len; + size_t len; - len = strlen(what); - pos[len] = '\0'; + len = strlen(what); + pos[-1] = '\0'; return pos + len + 1; } @@ -462,6 +464,7 @@ parse_indexdef(IndexDef *stmt, Oid index, Oid table) char *sql = pg_get_indexdef_string(index); const char *idxname = get_quoted_relname(index); const char *tblname = get_relation_name(table); + const char *limit = strchr(sql, '\0'); /* CREATE [UNIQUE] INDEX */ stmt->create = sql; @@ -486,8 +489,36 @@ parse_indexdef(IndexDef *stmt, Oid index, Oid table) stmt->columns = sql; if ((sql = skip_until(index, sql, ')')) == NULL) parse_error(index); + /* options */ stmt->options = sql; + stmt->tablespace = NULL; + stmt->where = NULL; + + /* Is there a tablespace? Note that apparently there is never, but + * if there was one it would appear here. */ + if (sql < limit && strstr(sql, "TABLESPACE")) + { + sql = skip_until_const(index, sql, "TABLESPACE"); + stmt->tablespace = sql; + sql = skip_ident(index, sql); + } + + /* Note: assuming WHERE is the only clause allowed after TABLESPACE */ + if (sql < limit && strstr(sql, "WHERE")) + { + sql = skip_until_const(index, sql, "WHERE"); + stmt->where = sql; + } + + elog(DEBUG2, "indexdef.create = %s", stmt->create); + elog(DEBUG2, "indexdef.index = %s", stmt->index); + elog(DEBUG2, "indexdef.table = %s", stmt->table); + elog(DEBUG2, "indexdef.type = %s", stmt->type); + elog(DEBUG2, "indexdef.columns = %s", stmt->columns); + elog(DEBUG2, "indexdef.options = %s", stmt->options); + elog(DEBUG2, "indexdef.tspace = %s", stmt->tablespace); + elog(DEBUG2, "indexdef.where = %s", stmt->where); } /* @@ -546,12 +577,6 @@ repack_get_order_by(PG_FUNCTION_ARGS) int nattr; parse_indexdef(&stmt, index, table); - elog(DEBUG2, "indexdef.create = %s", stmt.create); - elog(DEBUG2, "indexdef.index = %s", stmt.index); - elog(DEBUG2, "indexdef.table = %s", stmt.table); - elog(DEBUG2, "indexdef.type = %s", stmt.type); - elog(DEBUG2, "indexdef.columns = %s", stmt.columns); - elog(DEBUG2, "indexdef.options = %s", stmt.options); /* * FIXME: this is very unreliable implementation but I don't want to @@ -660,36 +685,16 @@ repack_indexdef(PG_FUNCTION_ARGS) parse_indexdef(&stmt, index, table); initStringInfo(&str); - appendStringInfo(&str, "%s index_%u ON repack.table_%u USING %s (%s)", - stmt.create, index, table, stmt.type, stmt.columns); + appendStringInfo(&str, "%s index_%u ON repack.table_%u USING %s (%s)%s", + stmt.create, index, table, stmt.type, stmt.columns, stmt.options); - /* Replace the tablespace in the index options */ - if (tablespace == NULL) - { - /* tablespace is just fine */ - appendStringInfoString(&str, stmt.options); - } - else - { - if (NULL == strstr(stmt.options, "TABLESPACE")) - { - /* tablespace is to append */ - appendStringInfoString(&str, " TABLESPACE "); - appendStringInfoString(&str, NameStr(*tablespace)); - } - else - { - /* tablespace is to replace */ - char *tmp, *limit; - limit = strchr(stmt.options, '\0'); - tmp = skip_until_const(index, stmt.options, " TABLESPACE"); - appendStringInfoString(&str, stmt.options); - appendStringInfo(&str, " %s", NameStr(*tablespace)); - tmp = skip_ident(index, tmp); - if (tmp < limit) - appendStringInfo(&str, " %s", tmp); - } - } + /* specify the new tablespace or the original one if any */ + if (tablespace || stmt.tablespace) + appendStringInfo(&str, " TABLESPACE %s", + (tablespace ? NameStr(*tablespace) : stmt.tablespace)); + + if (stmt.where) + appendStringInfo(&str, " WHERE %s", stmt.where); PG_RETURN_TEXT_P(cstring_to_text(str.data)); }