Merge remote-tracking branch 'piro/master'

.gitignore

@@ -1,3 +1,5 @@
 # Global excludes across all subdirectories
 *.o
 *.so
+bin/regression.diffs
+bin/regression.out

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 <schmiddy@gmail.com>",
        "Daniele Varrazzo <daniele.varrazzo@gmail.com>"
@@ -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"
       }
    },

Makefile

@@ -6,22 +6,25 @@
 #  Portions Copyright (c) 2012, The Reorg Development Team
 #
 
-USE_PGXS = 1
+PG_CONFIG ?= pg_config
-PG_CONFIG = pg_config
-PGXS := $(shell $(PG_CONFIG) --pgxs)
-include $(PGXS)
-
-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}')
+ifeq ("$(VERSION)","")
+$(error pg_config not found)
+endif
+
+# 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.
-ifneq ($(shell echo $(VERSION) | grep -E "^7\.|^8\.[012]"),)
+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; \

bin/Makefile

@@ -5,12 +5,40 @@
 #  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 -> 901
+VERSION := $(shell $(PG_CONFIG) --version | awk '{print $$2}')
+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)
 PROGRAM = pg_repack
-REGRESS = init repack
 
-EXTRA_CLEAN = sql/init-$(MAJORVERSION).sql sql/init.sql
+
+#
+# Test suite
+#
+
+ifeq ($(shell echo $$(($(INTVERSION) >= 901))),1)
+REGRESS := init-extension
+else
+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
+
+# This test depends on collate, not supported before 9.1
+ifeq ($(shell echo $$(($(INTVERSION) >= 901))),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 \
@@ -33,25 +61,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

bin/expected/init-extension.out

@@ -0,0 +1,3 @@
+SET client_min_messages = warning;
+CREATE EXTENSION pg_repack;
+RESET client_min_messages;

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"

bin/expected/plpgsql.out

@@ -0,0 +1 @@
+CREATE LANGUAGE plpgsql;

bin/expected/repack.out

@@ -114,10 +114,20 @@ SELECT * FROM tbl_with_dropped_toast;
 --
 -- do repack
 --
-\! pg_repack --dbname=contrib_regression --no-order
+\! 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
-\! pg_repack --dbname=contrib_regression --table=tbl_cluster
+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"
+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
 --
@@ -301,64 +311,44 @@ 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
+INFO: repacking 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
+INFO: repacking 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
 --
--- pg_repack issue #3
+-- Triggers handling
 --
-CREATE TABLE issue3_1 (col1 int NOT NULL, col2 text NOT NULL);
+CREATE FUNCTION trgtest() RETURNS trigger AS
-CREATE UNIQUE INDEX issue3_1_idx ON issue3_1 (col1, col2 DESC);
+$$BEGIN RETURN NEW; END$$
-SELECT repack.get_order_by('issue3_1_idx'::regclass::oid, 'issue3_1'::regclass::oid);
+LANGUAGE plpgsql;
-  get_order_by   
+CREATE TABLE trg1 (id integer PRIMARY KEY);
------------------
+CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest();
- col1, col2 DESC
+\! pg_repack --dbname=contrib_regression --table=trg1
-(1 row)
+INFO: repacking table "trg1"
-
+CREATE TABLE trg2 (id integer PRIMARY KEY);
-\! pg_repack --dbname=contrib_regression --no-order --table=issue3_1
+CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest();
-CREATE TABLE issue3_2 (col1 int NOT NULL, col2 text NOT NULL);
+\! pg_repack --dbname=contrib_regression --table=trg2
-CREATE UNIQUE INDEX issue3_2_idx ON issue3_2 (col1 DESC, col2 text_pattern_ops);
+INFO: repacking table "trg2"
-SELECT repack.get_order_by('issue3_2_idx'::regclass::oid, 'issue3_2'::regclass::oid);
+WARNING: the table "trg2" has already a trigger called "z_repack_trigger"
-       get_order_by        
+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);
- col1 DESC, col2 USING ~<~
+CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest();
-(1 row)
+\! pg_repack --dbname=contrib_regression --table=trg3
-
+INFO: repacking table "trg3"
-\! pg_repack --dbname=contrib_regression --no-order --table=issue3_2
+WARNING: trigger "z_repack_trigges" conflicting on table "trg3"
-CREATE TABLE issue3_3 (col1 int NOT NULL, col2 text NOT NULL);
+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 UNIQUE INDEX issue3_3_idx ON issue3_3 (col1 DESC, col2 DESC);
+CREATE TABLE trg4 (id integer PRIMARY KEY);
-SELECT repack.get_order_by('issue3_3_idx'::regclass::oid, 'issue3_3'::regclass::oid);
+CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest();
-     get_order_by     
+\! pg_repack --dbname=contrib_regression --table=trg4
-----------------------
+INFO: repacking table "trg4"
- col1 DESC, col2 DESC
-(1 row)
-
-\! pg_repack --dbname=contrib_regression --no-order --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 --no-order --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 --no-order --table=issue3_5

bin/expected/tablespace.out

@@ -0,0 +1,110 @@
+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);
+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');
+-- 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"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+ 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
+INFO: repacking table "testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+ 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
+INFO: repacking table "testts1"
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+ relname | spcname 
+---------+---------
+(0 rows)
+
+-- 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'
+ORDER BY relname;
+       relname       | spcname 
+---------------------+---------
+ testts1             | testts
+ testts1_partial_idx | testts
+ testts1_pkey        | testts
+ testts1_with_idx    | testts
+(4 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)
+-- not broken with order
+\! pg_repack --dbname=contrib_regression -o id --table=testts1 --tablespace pg_default --moveidx
+INFO: repacking table "testts1"

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 : \
@@ -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);
@@ -185,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)
@@ -197,6 +199,8 @@ static bool				alldb = false;
 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. */
 
@@ -214,6 +218,8 @@ static pgut_option options[] =
 	{ 'l', 't', "table", &table_list },
 	{ '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 },
@@ -234,6 +240,8 @@ main(int argc, char *argv[])
 			(errcode(EINVAL),
 			 errmsg("too many arguments")));
 
+	check_tablespace();
+
 	if (noorder)
 		orderby = "";
 
@@ -281,6 +289,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.
@@ -307,22 +365,10 @@ repack_all_databases(const char *orderby)
 
 		dbname = PQgetvalue(result, i, 0);
 
-		if (pgut_log_level >= INFO)
+		elog(INFO, "repacking database \"%s\"", dbname);
-		{
-			printf("%s: repack database \"%s\"\n", PROGRAM_NAME, dbname);
-			fflush(stdout);
-		}
-
 		ret = repack_one_database(orderby, errbuf, sizeof(errbuf));
-
+		if (!ret)
-		if (pgut_log_level >= INFO)
+			elog(INFO, "database \"%s\" skipped: %s", dbname, errbuf);
-		{
-			if (ret)
-				printf("\n");
-			else
-				printf(" ... skipped: %s\n", errbuf);
-			fflush(stdout);
-		}
 	}
 
 	CLEARPGRES(result);
@@ -360,10 +406,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)
+	num_tables = simple_string_list_size(table_list);
-		params = pgut_malloc(num_params * sizeof(char *));
+
+	/* 1st param is the user-specified tablespace */
+	num_params = num_tables + 1;
+	params = pgut_malloc(num_params * sizeof(char *));
 
 	initStringInfo(&sql);
 
@@ -415,12 +466,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
 		{
@@ -442,45 +497,50 @@ 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 ");
+	appendStringInfoString(&sql,
-	if (num_params)
+		"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);
+			appendStringInfo(&sql, "relid = $%d::regclass", iparam + 1);
-			params[i] = cell->val;
+			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)
 	{
-		if (sqlstate_equals(res, SQLSTATE_INVALID_SCHEMA_NAME))
+		/* Return the error message otherwise */
-		{
+		if (errbuf)
-			/* Schema repack does not exist. Skip the database. */
+			snprintf(errbuf, errsize, "%s", PQerrorMessage(connection));
-			if (errbuf)
-				snprintf(errbuf, errsize,
-						 "%s is not installed in the database", PROGRAM_NAME);
-		}
-		else
-		{
-			/* Return the error message otherwise */
-			if (errbuf)
-				snprintf(errbuf, errsize, "%s", PQerrorMessage(connection));
-		}
 		goto cleanup;
 	}
 
@@ -489,7 +549,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,43 +574,51 @@ 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++);
-
-		resetStringInfo(&sql);
-		if (!orderby)
-		{
-			/* CLUSTER mode */
-			if (ckey == NULL)
-			{
-				ereport(WARNING,
-					(errcode(E_PG_COMMAND),
-					 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;
-		}
-		else if (!orderby[0])
-		{
-			/* VACUUM FULL mode */
-            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.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)
+		{
+			if (ckey != NULL)
+			{
+				/* CLUSTER mode */
+				appendStringInfoString(&sql, " ORDER BY ");
+				appendStringInfoString(&sql, ckey);
+				table.create_table = sql.data;
+			}
+			else
+			{
+				/* VACUUM FULL mode (non-clustered tables) */
+				table.create_table = sql.data;
+			}
+		}
+		else if (!orderby[0])
+		{
+			/* VACUUM FULL mode (for clustered tables too) */
+			table.create_table = sql.data;
+		}
+		else
+		{
+			/* User specified ORDER BY */
+			appendStringInfoString(&sql, " ORDER BY ");
+			appendStringInfoString(&sql, orderby);
+			table.create_table = sql.data;
+		}
 
 		repack_one_table(&table, orderby);
 	}
@@ -593,7 +663,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;
@@ -605,6 +675,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).
@@ -620,8 +691,9 @@ rebuild_indexes(const repack_table *table)
 	}
 
 	res = execute("SELECT indexrelid,"
-		" repack.repack_indexdef(indexrelid, indrelid) "
+		" repack.repack_indexdef(indexrelid, indrelid, $2) "
-		" FROM pg_index WHERE indrelid = $1 AND indisvalid", 1, params);
+		" FROM pg_index WHERE indrelid = $1 AND indisvalid",
+		2, params);
 
 	num_indexes = PQntuples(res);
 
@@ -845,6 +917,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);
@@ -886,10 +960,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,
+		if (0 == strcmp("z_repack_trigger", PQgetvalue(res, 0, 0)))
-			(errcode(E_PG_COMMAND),
+		{
-			 errmsg("trigger %s conflicted for %s",
+			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;
 	}
@@ -914,12 +1013,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);
@@ -1161,6 +1254,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().
 	 */
@@ -1399,10 +1496,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)
@@ -1426,10 +1519,12 @@ 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("  -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");
 	printf("  -T, --wait-timeout=SECS   timeout to cancel other backends on conflict\n");
 	printf("  -Z, --no-analyze          don't analyze at end\n");
 }

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;

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

bin/sql/plpgsql.sql

@@ -0,0 +1 @@
+CREATE LANGUAGE plpgsql;

bin/sql/repack.sql

@@ -120,9 +120,9 @@ SELECT * FROM tbl_with_dropped_toast;
 -- do repack
 --
 
-\! pg_repack --dbname=contrib_regression --no-order
-\! pg_repack --dbname=contrib_regression
 \! pg_repack --dbname=contrib_regression --table=tbl_cluster
+\! pg_repack --dbname=contrib_regression --table=tbl_badindex
+\! pg_repack --dbname=contrib_regression
 
 --
 -- after
@@ -177,41 +177,32 @@ 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
 
 --
--- pg_repack issue #3
+-- Triggers handling
 --
-CREATE TABLE issue3_1 (col1 int NOT NULL, col2 text NOT NULL);
+CREATE FUNCTION trgtest() RETURNS trigger AS
-CREATE UNIQUE INDEX issue3_1_idx ON issue3_1 (col1, col2 DESC);
+$$BEGIN RETURN NEW; END$$
-SELECT repack.get_order_by('issue3_1_idx'::regclass::oid, 'issue3_1'::regclass::oid);
+LANGUAGE plpgsql;
-\! pg_repack --dbname=contrib_regression --no-order --table=issue3_1
+CREATE TABLE trg1 (id integer PRIMARY KEY);
-
+CREATE TRIGGER z_repack_triggeq BEFORE UPDATE ON trg1 FOR EACH ROW EXECUTE PROCEDURE trgtest();
-CREATE TABLE issue3_2 (col1 int NOT NULL, col2 text NOT NULL);
+\! pg_repack --dbname=contrib_regression --table=trg1
-CREATE UNIQUE INDEX issue3_2_idx ON issue3_2 (col1 DESC, col2 text_pattern_ops);
+CREATE TABLE trg2 (id integer PRIMARY KEY);
-SELECT repack.get_order_by('issue3_2_idx'::regclass::oid, 'issue3_2'::regclass::oid);
+CREATE TRIGGER z_repack_trigger BEFORE UPDATE ON trg2 FOR EACH ROW EXECUTE PROCEDURE trgtest();
-\! pg_repack --dbname=contrib_regression --no-order --table=issue3_2
+\! pg_repack --dbname=contrib_regression --table=trg2
-
+CREATE TABLE trg3 (id integer PRIMARY KEY);
-CREATE TABLE issue3_3 (col1 int NOT NULL, col2 text NOT NULL);
+CREATE TRIGGER z_repack_trigges BEFORE UPDATE ON trg3 FOR EACH ROW EXECUTE PROCEDURE trgtest();
-CREATE UNIQUE INDEX issue3_3_idx ON issue3_3 (col1 DESC, col2 DESC);
+\! pg_repack --dbname=contrib_regression --table=trg3
-SELECT repack.get_order_by('issue3_3_idx'::regclass::oid, 'issue3_3'::regclass::oid);
+CREATE TABLE trg4 (id integer PRIMARY KEY);
-\! pg_repack --dbname=contrib_regression --no-order --table=issue3_3
+CREATE TRIGGER zzzzzz AFTER UPDATE ON trg4 FOR EACH ROW EXECUTE PROCEDURE trgtest();
-
+\! pg_repack --dbname=contrib_regression --table=trg4
-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
-
-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

bin/sql/tablespace.sql

@@ -0,0 +1,71 @@
+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);
+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');
+
+-- 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
+
+SELECT relname, spcname
+FROM pg_class JOIN pg_tablespace ts ON ts.oid = reltablespace
+WHERE relname ~ '^testts1'
+ORDER BY relname;
+
+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'
+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'
+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'
+ORDER BY relname;
+
+-- 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
+
+-- not broken with order
+\! pg_repack --dbname=contrib_regression -o id --table=testts1 --tablespace pg_default --moveidx

doc/pg_repack.rst

@@ -117,10 +117,12 @@ 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
+  -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
   -T, --wait-timeout=SECS   timeout to cancel other backends on conflict
   -Z, --no-analyze          don't analyze at end
 
@@ -142,26 +144,36 @@ Generic options:
 Reorg Options
 ^^^^^^^^^^^^^
 
-Options to order rows. If not specified, pg_repack performs an online CLUSTER
+``-a``, ``--all``
-using cluster indexes. Only one option can be specified. You may also specify
+    Attempt repack all the databases of the cluster. Databases where the
-target tables or databases.
+    ``pg_repack`` extension is not installed will be skipped.
-
-``-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.
 
 ``-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.
+
+``-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.
+
+``-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 +187,7 @@ target tables or databases.
     Disable ANALYZE after the reorganization. If not specified, run ANALYZE
     after the reorganization.
 
+
 Connection Options
 ^^^^^^^^^^^^^^^^^^
 
@@ -253,13 +266,13 @@ Environment
 Examples
 --------
 
-Execute the following command to perform an online CLUSTER of all tables in
+Perform an online CLUSTER of all the clustered tables in the database
-database ``test``::
+``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
+Perform an online VACUUM FULL on the table ``foo`` in the database ``test``
-``foo`` in database ``test``::
+(an eventual cluster index is ignored)::
 
     $ pg_repack --no-order --table foo -d test
 
@@ -280,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
+INFO: database "db" skipped: pg_repack VER is not installed in the database
-    pg_repack is not installed in the database when ``--all`` option is
+    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.
@@ -313,37 +326,28 @@ 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
+ERROR: query failed: ERROR: column "col" does not exist
-    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.
-
-pg_repack: 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
+WARNING: the table "tbl" has already a trigger called z_repack_trigger
-    Permission error.
+    The trigger was probably installed during a previous attempt to run
-
+    pg_repack on the table which was interrupted and for some reason failed
-    pg_repack must be executed by a superuser.
+    to clean up the temporary objects.
-
-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.
 
     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
@@ -399,6 +403,17 @@ and the original one.
 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).
+  * 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
 
   * Added support for PostgreSQL 9.2.

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 -> 901
+VERSION := $(shell $(PG_CONFIG) --version | awk '{print $$2}')
+INTVERSION := $(shell echo $$(($$(echo $(VERSION) | sed -E 's/([0-9]+)\.([0-9]+).*/\1*100+\2/'))))
 
 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 \
+ifeq ($(shell echo $$(($(INTVERSION) >= 901))),1)
-	| grep -qE " 8\.| 9\.0" && echo no || echo yes)
-
-ifeq ($(HAVE_EXTENSION),yes)
 DATA_built = pg_repack--$(REPACK_VERSION).sql pg_repack.control
 else
 DATA_built = pg_repack.sql

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') 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,
          'LOCK TABLE ' || repack.oid2text(R.oid) || ' IN ACCESS EXCLUSIVE MODE' AS lock_table,
@@ -205,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'
@@ -217,6 +219,8 @@ 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;
 

lib/repack.c

@@ -307,7 +307,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 *
@@ -348,6 +350,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[-1] = '\0';
+		return pos + len + 1;
+	}
+
+	/* error */
+	return parse_error(index);
+}
+
 static char *
 skip_ident(Oid index, char *sql)
 {
@@ -446,6 +466,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;
@@ -470,8 +491,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);
 }
 
 /*
@@ -530,12 +579,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
@@ -620,21 +663,41 @@ 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				index;
-	Oid				table = PG_GETARG_OID(1);
+	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);
 
+	/* 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));
 }