From 20beaf99f276ad5f161ab0ef099e693677fe9400 Mon Sep 17 00:00:00 2001 From: Dmitry Ivanov Date: Mon, 24 Oct 2016 16:11:12 +0300 Subject: [PATCH 1/6] introduce new option 'include-extensions' which is aimed to prevent pg_repack from touching tables that belong to extensions --- bin/pg_repack.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 8ccb1ae..279d5e8 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -109,7 +109,7 @@ const char *PROGRAM_VERSION = "unknown"; */ #define SQL_XID_SNAPSHOT_80300 \ "SELECT repack.array_accum(l.virtualtransaction) " \ - " FROM pg_locks AS l" \ + " FROM pg_locks AS l" \ " LEFT JOIN pg_stat_activity AS a " \ " ON l.pid = a.procpid " \ " LEFT JOIN pg_database AS d " \ @@ -169,7 +169,7 @@ typedef struct repack_index { Oid target_oid; /* target: OID */ const char *create_index; /* CREATE INDEX */ - index_status_t status; /* Track parallel build statuses. */ + index_status_t status; /* Track parallel build statuses. */ int worker_idx; /* which worker conn is handling */ } repack_index; @@ -244,6 +244,7 @@ static int wait_timeout = 60; /* in seconds */ static int jobs = 0; /* number of concurrent worker conns. */ static bool dryrun = false; static unsigned int temp_obj_num = 0; /* temporary objects counter */ +static bool include_extensions = false; /* repack tables of extensions */ /* buffer should have at least 11 bytes */ static char * @@ -269,6 +270,7 @@ static pgut_option options[] = { 'i', 'T', "wait-timeout", &wait_timeout }, { 'B', 'Z', "no-analyze", &analyze }, { 'i', 'j', "jobs", &jobs }, + { 'b', 'C', "include-extensions", &include_extensions }, { 0 }, }; @@ -651,6 +653,16 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) { appendStringInfoString(&sql, "pkid IS NOT NULL"); } + + /* Exclude tables which belong to extensions */ + if (!include_extensions) + { + appendStringInfoString(&sql, " AND t.relid NOT IN" + " (SELECT objid FROM pg_depend" + " WHERE refclassid = 'pg_extension'::regclass" + " AND classid = 'pg_class'::regclass)"); + } + /* Ensure the regression tests get a consistent ordering of tables */ appendStringInfoString(&sql, " ORDER BY t.relname, t.schemaname"); @@ -2064,4 +2076,5 @@ pgut_help(bool details) printf(" -x, --only-indexes move only indexes of the specified 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"); + printf(" -C, --include-extensions repack tables which belong to extensions\n"); } From 116fa68097b268a64e2564fdd36528ca557d2aca Mon Sep 17 00:00:00 2001 From: Dmitry Ivanov Date: Mon, 24 Oct 2016 16:54:32 +0300 Subject: [PATCH 2/6] set 'include_extensions' to true if tables have been specified --- bin/pg_repack.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 279d5e8..ca75429 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -625,6 +625,12 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) params[iparam++] = tablespace; if (num_tables) { + /* + * Tables have been explicitly specified, + * no need to hide tables of extensions + */ + include_extensions = true; + appendStringInfoString(&sql, "("); for (cell = table_list.head; cell; cell = cell->next) { From 1ecec8ce95fae6f6ade04cbbc33ebf9ce38affec Mon Sep 17 00:00:00 2001 From: Dmitry Ivanov Date: Thu, 23 Mar 2017 15:24:41 +0300 Subject: [PATCH 3/6] change option to '--exclude-extension' --- bin/pg_repack.c | 37 +++++++++++++++++++++++-------------- 1 file changed, 23 insertions(+), 14 deletions(-) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index 308fc36..a996a0d 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -251,7 +251,7 @@ static bool dryrun = false; static unsigned int temp_obj_num = 0; /* temporary objects counter */ static bool no_kill_backend = false; /* abandon when timed-out */ static bool no_superuser_check = false; -static bool include_extensions = false; /* repack tables of extensions */ +static SimpleStringList exclude_extension_list = {NULL, NULL}; /* don't repack tables of these extensions */ /* buffer should have at least 11 bytes */ static char * @@ -279,7 +279,7 @@ static pgut_option options[] = { 'i', 'j', "jobs", &jobs }, { 'b', 'D', "no-kill-backend", &no_kill_backend }, { 'b', 'k', "no-superuser-check", &no_superuser_check }, - { 'b', 'C', "include-extensions", &include_extensions }, + { 'l', 'C', "exclude-extension", &exclude_extension_list }, { 0 }, }; @@ -607,12 +607,14 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) size_t num_tables; size_t num_schemas; size_t num_params; + size_t num_excluded_extensions; num_tables = simple_string_list_size(table_list); num_schemas = simple_string_list_size(schema_list); + num_excluded_extensions = simple_string_list_size(exclude_extension_list); /* 1st param is the user-specified tablespace */ - num_params = num_tables + num_schemas + 1; + num_params = num_excluded_extensions + num_tables + num_schemas + 1; params = pgut_malloc(num_params * sizeof(char *)); initStringInfo(&sql); @@ -637,12 +639,6 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) params[iparam++] = tablespace; if (num_tables) { - /* - * Tables have been explicitly specified, - * no need to hide tables of extensions - */ - include_extensions = true; - appendStringInfoString(&sql, "("); for (cell = table_list.head; cell; cell = cell->next) { @@ -673,12 +669,25 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize) } /* Exclude tables which belong to extensions */ - if (!include_extensions) + if (exclude_extension_list.head) { appendStringInfoString(&sql, " AND t.relid NOT IN" - " (SELECT objid FROM pg_depend" - " WHERE refclassid = 'pg_extension'::regclass" - " AND classid = 'pg_class'::regclass)"); + " (SELECT d.objid::regclass" + " FROM pg_depend d JOIN pg_extension e" + " ON d.refobjid = e.oid" + " WHERE d.classid = 'pg_class'::regclass AND ("); + + /* List all excluded extensions */ + for (cell = exclude_extension_list.head; cell; cell = cell->next) + { + appendStringInfo(&sql, "e.extname = $%d", iparam + 1); + params[iparam++] = cell->val; + + appendStringInfoString(&sql, cell->next ? " OR " : ")"); + } + + /* Close subquery */ + appendStringInfoString(&sql, ")"); } /* Ensure the regression tests get a consistent ordering of tables */ @@ -2118,5 +2127,5 @@ pgut_help(bool details) printf(" -D, --no-kill-backend don't kill other backends when timed out\n"); printf(" -Z, --no-analyze don't analyze at end\n"); printf(" -k, --no-superuser-check skip superuser checks in client\n"); - printf(" -C, --include-extensions repack tables which belong to extensions\n"); + printf(" -C, --exclude-extension don't repack tables which belong to specific extension\n"); } From e1f41aa6d805ed0a7018c6c09f89f61c22907dca Mon Sep 17 00:00:00 2001 From: Dmitry Ivanov Date: Thu, 23 Mar 2017 16:05:59 +0300 Subject: [PATCH 4/6] add new option checks for -C --- bin/pg_repack.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/bin/pg_repack.c b/bin/pg_repack.c index a996a0d..fc7b039 100644 --- a/bin/pg_repack.c +++ b/bin/pg_repack.c @@ -311,9 +311,15 @@ main(int argc, char *argv[]) else if (r_index.head && only_indexes) ereport(ERROR, (errcode(EINVAL), errmsg("cannot specify --index (-i) and --only-indexes (-x)"))); + else if (r_index.head && exclude_extension_list.head) + ereport(ERROR, (errcode(EINVAL), + errmsg("cannot specify --index (-i) and --exclude-extension (-C)"))); else if (only_indexes && !table_list.head) ereport(ERROR, (errcode(EINVAL), errmsg("cannot repack all indexes of database, specify the table(s) via --table (-t)"))); + else if (only_indexes && exclude_extension_list.head) + ereport(ERROR, (errcode(EINVAL), + errmsg("cannot specify --only-indexes (-x) and --exclude-extension (-C)"))); else if (alldb) ereport(ERROR, (errcode(EINVAL), errmsg("cannot repack specific index(es) in all databases"))); @@ -343,6 +349,11 @@ main(int argc, char *argv[]) (errcode(EINVAL), errmsg("cannot repack specific table(s) in schema, use schema.table notation instead"))); + if (exclude_extension_list.head && table_list.head) + ereport(ERROR, + (errcode(EINVAL), + errmsg("cannot specify --table (-t) and --exclude-extension (-C)"))); + if (noorder) orderby = ""; From c62488ae1363d2a545e49f386864353c41ac13d9 Mon Sep 17 00:00:00 2001 From: Dmitry Ivanov Date: Thu, 23 Mar 2017 16:26:46 +0300 Subject: [PATCH 5/6] add regression tests for --exclude-extension (-C) --- regress/expected/repack.out | 20 ++++++++++++++++++++ regress/sql/repack.sql | 16 ++++++++++++++++ 2 files changed, 36 insertions(+) diff --git a/regress/expected/repack.out b/regress/expected/repack.out index 17e3666..4539043 100644 --- a/regress/expected/repack.out +++ b/regress/expected/repack.out @@ -403,3 +403,23 @@ ERROR: pg_repack failed with error: ERROR: permission denied for schema repack LINE 1: select repack.version(), repack.version_sql() ^ DROP ROLE IF EXISTS nosuper; +-- +-- exclude extension check +-- +CREATE SCHEMA exclude_extension_schema; +CREATE TABLE exclude_extension_schema.tbl(val integer primary key); +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=dummy_table --exclude-extension=dummy_extension +ERROR: cannot specify --table (-t) and --exclude-extension (-C) +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=dummy_table --exclude-extension=dummy_extension -x +ERROR: cannot specify --only-indexes (-x) and --exclude-extension (-C) +-- => ERROR +\! pg_repack --dbname=contrib_regression --index=dummy_index --exclude-extension=dummy_extension +ERROR: cannot specify --index (-i) and --exclude-extension (-C) +-- => OK +\! pg_repack --dbname=contrib_regression --schema=exclude_extension_schema --exclude-extension=dummy_extension +INFO: repacking table "exclude_extension_schema.tbl" +-- => OK +\! pg_repack --dbname=contrib_regression --schema=exclude_extension_schema --exclude-extension=dummy_extension --exclude-extension=dummy_extension +INFO: repacking table "exclude_extension_schema.tbl" diff --git a/regress/sql/repack.sql b/regress/sql/repack.sql index e613d63..d7baa7d 100644 --- a/regress/sql/repack.sql +++ b/regress/sql/repack.sql @@ -245,3 +245,19 @@ CREATE ROLE nosuper WITH LOGIN; -- => ERROR \! pg_repack --dbname=contrib_regression --table=tbl_cluster --username=nosuper --no-superuser-check DROP ROLE IF EXISTS nosuper; + +-- +-- exclude extension check +-- +CREATE SCHEMA exclude_extension_schema; +CREATE TABLE exclude_extension_schema.tbl(val integer primary key); +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=dummy_table --exclude-extension=dummy_extension +-- => ERROR +\! pg_repack --dbname=contrib_regression --table=dummy_table --exclude-extension=dummy_extension -x +-- => ERROR +\! pg_repack --dbname=contrib_regression --index=dummy_index --exclude-extension=dummy_extension +-- => OK +\! pg_repack --dbname=contrib_regression --schema=exclude_extension_schema --exclude-extension=dummy_extension +-- => OK +\! pg_repack --dbname=contrib_regression --schema=exclude_extension_schema --exclude-extension=dummy_extension --exclude-extension=dummy_extension From 34194227861842b1a7007b4909869ca49ed207e6 Mon Sep 17 00:00:00 2001 From: Dmitry Ivanov Date: Thu, 23 Mar 2017 16:36:54 +0300 Subject: [PATCH 6/6] update docs --- doc/pg_repack.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/doc/pg_repack.rst b/doc/pg_repack.rst index 2205cbb..872629a 100644 --- a/doc/pg_repack.rst +++ b/doc/pg_repack.rst @@ -130,6 +130,7 @@ Options: -D, --no-kill-backend don't kill other backends when timed out -Z, --no-analyze don't analyze at end -k, --no-superuser-check skip superuser checks in client + -C, --exclude-extension don't repack tables which belong to specific extension Connection options: -d, --dbname=DBNAME database to connect @@ -222,6 +223,9 @@ Reorg Options Skip the superuser checks in the client. This setting is useful for using pg_repack on platforms that support running it as non-superusers. +``-C``, ``--exclude-extension`` + Skip tables that belong to the specified extension(s). Some extensions + may heavily depend on such tables at planning time etc. Connection Options ^^^^^^^^^^^^^^^^^^