From 6710e514db8f590e4c9d980300f7642bc146502a Mon Sep 17 00:00:00 2001 From: Daniele Varrazzo Date: Thu, 21 Feb 2013 17:10:12 +0000 Subject: [PATCH 1/7] 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 2/7] 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 3/7] 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 4/7] 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 5/7] 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 6/7] 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 7/7] 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.