Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for exclude table (-X) and exclude parent table (-Y) option #360

Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

Support for exclude table (-X) and exclude parent table (-Y) option #360

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
2a2bf64
initial commit for --exclude-table -X
May 28, 2023
5ee5bac
initial commit for --exclude-table -X option
May 28, 2023
950f338
Merge remote-tracking branch 'origin/support-----exclude-table-option…
May 28, 2023
835f49b
adding --exclude-parent-table option
May 29, 2023
f86e852
meaning full info for options
May 29, 2023
740a7ce
adding necessary checks
May 29, 2023
e51501d
if condition checking split
Jun 21, 2023
2ecdf37
clearing white spaces
Jul 18, 2023
9a0ce6d
providing only long option names
Jul 18, 2023
62449b6
documentation update
Jul 18, 2023
a3e32bd
code cleanup
Jul 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 95 additions & 5 deletions bin/pg_repack.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,8 @@ static bool alldb = false;
static bool noorder = false;
static SimpleStringList parent_table_list = {NULL, NULL};
static SimpleStringList table_list = {NULL, NULL};
static SimpleStringList exclude_table_list = {NULL, NULL};
static SimpleStringList exclude_parent_table_list = {NULL, NULL};
static SimpleStringList schema_list = {NULL, NULL};
static char *orderby = NULL;
static char *tablespace = NULL;
Expand Down Expand Up @@ -287,6 +289,8 @@ static pgut_option options[] =
{ 'b', 'D', "no-kill-backend", &no_kill_backend },
{ 'b', 'k', "no-superuser-check", &no_superuser_check },
{ 'l', 'C', "exclude-extension", &exclude_extension_list },
{ 'l', 'X', "exclude-table", &exclude_table_list },
{ 'l', 'Y', "exclude-parent-table", &exclude_parent_table_list },
{ 'b', 2, "error-on-invalid-index", &error_on_invalid_index },
{ 'i', 1, "switch-threshold", &switch_threshold },
{ 0 },
Expand Down Expand Up @@ -333,6 +337,9 @@ main(int argc, char *argv[])
else if (only_indexes && exclude_extension_list.head)
ereport(ERROR, (errcode(EINVAL),
errmsg("cannot specify --only-indexes (-x) and --exclude-extension (-C)")));
else if ((only_indexes || r_index.head) && exclude_table_list.head)
ereport(ERROR, (errcode(EINVAL),
errmsg("cannot specify --only-indexes (-x) or --index (-i) with --exclude-table (-X)")));
else if (alldb)
ereport(ERROR, (errcode(EINVAL),
errmsg("cannot repack specific index(es) in all databases")));
Expand All @@ -357,11 +364,26 @@ main(int argc, char *argv[])
}
else
{
if (schema_list.head && (table_list.head || parent_table_list.head))
if (schema_list.head && (table_list.head || parent_table_list.head ))
za-arthur marked this conversation as resolved.
Show resolved Hide resolved
ereport(ERROR,
(errcode(EINVAL),
errmsg("cannot repack specific table(s) in schema, use schema.table notation instead")));

if (schema_list.head && ( exclude_table_list.head || exclude_parent_table_list.head ))
ereport(ERROR,
(errcode(EINVAL),
errmsg("cannot exclude specific table(s) in schema, use schema.table notation instead")));

if (exclude_table_list.head && (table_list.head || parent_table_list.head || exclude_extension_list.head))
ereport(ERROR,
(errcode(EINVAL),
errmsg("cannot specify --exclude-table (-X) along with --table (-t), --parent-table (-I) and --exclude-extension (-C)")));

if (exclude_parent_table_list.head && (table_list.head || parent_table_list.head || exclude_extension_list.head))
ereport(ERROR,
(errcode(EINVAL),
errmsg("cannot specify --exclude-parent-table (-Y) along with --table (-t), --parent-table (-I) and --exclude-extension (-C)")));

if (exclude_extension_list.head && table_list.head)
ereport(ERROR,
(errcode(EINVAL),
Expand All @@ -385,6 +407,10 @@ main(int argc, char *argv[])
ereport(ERROR,
(errcode(EINVAL),
errmsg("cannot repack specific schema(s) in all databases")));
if (exclude_table_list.head || exclude_parent_table_list.head)
ereport(ERROR,
(errcode(EINVAL),
errmsg("cannot exclude specific tables(s) in all databases")));
repack_all_databases(orderby);
}
else
Expand Down Expand Up @@ -579,7 +605,9 @@ is_requested_relation_exists(char *errbuf, size_t errsize){
SimpleStringListCell *cell;

num_relations = simple_string_list_size(parent_table_list) +
simple_string_list_size(table_list);
simple_string_list_size(table_list)+
simple_string_list_size(exclude_table_list)+
simple_string_list_size(exclude_parent_table_list);

/* nothing was implicitly requested, so nothing to do here */
if (num_relations == 0)
Expand All @@ -600,13 +628,27 @@ is_requested_relation_exists(char *errbuf, size_t errsize){
if (iparam < num_relations)
appendStringInfoChar(&sql, ',');
}
for (cell = exclude_table_list.head; cell; cell = cell->next)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic is probably unnecessary since exclude-table conflicts with table and parent-table:
https://github.com/reorg/pg_repack/pull/360/files#diff-a9d4d5b8968d339c50f089a6b531b1f36eedd27cdc008bdb26b70fb9fd0eee2fR377

Copy link
Author

@chanukyasds chanukyasds Jul 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic helps pg_repack to find the exclude-table exists in the database or not.

I agree that exlude-table conflicts with table and parent-table options.

For example, consider below case.

pg_repack -p 5433 -d postgres -X test.table1

In above command user can skip a table from a database ,so we need to check the exclude-table (test.table1) existence.

some other examples:

pg_repack -p 5433 -d postgres -X test.table1 -X public.table_1

pg_repack -p 5433 -d postgres -X test.table1 -Y public.ptable2

{
appendStringInfo(&sql, "($%d, 'r')", iparam + 1);
params[iparam++] = cell->val;
if (iparam < num_relations)
appendStringInfoChar(&sql, ',');
}
for (cell = parent_table_list.head; cell; cell = cell->next)
{
appendStringInfo(&sql, "($%d, 'p')", iparam + 1);
params[iparam++] = cell->val;
if (iparam < num_relations)
appendStringInfoChar(&sql, ',');
}
for (cell = exclude_parent_table_list.head; cell; cell = cell->next)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above exclude-parent-table conflicts with table and parent-table:
https://github.com/reorg/pg_repack/pull/360/files#diff-a9d4d5b8968d339c50f089a6b531b1f36eedd27cdc008bdb26b70fb9fd0eee2fR382

Copy link
Author

@chanukyasds chanukyasds Jul 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.

This logic helps pg_repack to find the exclude-parent-table exists in the database or not.

For example consider below case.

pg_repack -p 5433 -d postgres -Y public.ptable2

In above command user can skip a parent table from a database, so we need to check the exclude-parent-table (public.ptable2) existence.

{
appendStringInfo(&sql, "($%d, 'p')", iparam + 1);
params[iparam++] = cell->val;
if (iparam < num_relations)
appendStringInfoChar(&sql, ',');
}
appendStringInfoString(&sql,
") AS given_t(r,kind) WHERE"
/* regular --table relation or inherited --parent-table */
Expand Down Expand Up @@ -749,18 +791,26 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize)
num_tables,
num_schemas,
num_params,
num_excluded_extensions;
num_excluded_extensions,
num_excluded_tables,
num_excluded__parent_tables;



num_parent_tables = simple_string_list_size(parent_table_list);
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);
num_excluded_tables = simple_string_list_size(exclude_table_list);
num_excluded__parent_tables = simple_string_list_size(exclude_parent_table_list);

/* 1st param is the user-specified tablespace */
num_params = num_excluded_extensions +
num_parent_tables +
num_tables +
num_schemas + 1;
num_schemas +
num_excluded_tables +
num_excluded__parent_tables + 1;
params = pgut_malloc(num_params * sizeof(char *));

initStringInfo(&sql);
Expand Down Expand Up @@ -835,6 +885,44 @@ repack_one_database(const char *orderby, char *errbuf, size_t errsize)
appendStringInfoString(&sql, ", ");
}
appendStringInfoString(&sql, ")");
}
else if (num_excluded_tables || num_excluded__parent_tables)
{
if (exclude_table_list.head)
{
appendStringInfoString(&sql, "(");
for (cell = exclude_table_list.head; cell; cell = cell->next)
{
/* Construct exclude table name placeholders to be used by PQexecParams */
appendStringInfo(&sql, "relid <> $%d::regclass", iparam + 1);
params[iparam++] = cell->val;
elog(INFO, "skipping table \"%s\"", cell->val);
if (cell->next)
appendStringInfoString(&sql, " AND ");
}
appendStringInfoString(&sql, ")");
}
if (exclude_parent_table_list.head)
{
if (exclude_table_list.head)
appendStringInfoString(&sql, "AND (");
else
appendStringInfoString(&sql, " (");
for (cell = exclude_parent_table_list.head; cell; cell = cell->next)
{
/* Construct table name placeholders to be used by PQexecParams */
appendStringInfo(&sql,
"relid <> ALL(repack.get_table_and_inheritors($%d::regclass))",
iparam + 1);
params[iparam++] = cell->val;
elog(INFO, "skipping parent table \"%s\" with its inheritors", cell->val);
if (cell->next)
appendStringInfoString(&sql, " AND ");
}
appendStringInfoString(&sql, ")");
}


}
else
{
Expand Down Expand Up @@ -2392,6 +2480,8 @@ pgut_help(bool details)
printf(" -Z, --no-analyze don't analyze at end\n");
printf(" -k, --no-superuser-check skip superuser checks in client\n");
printf(" -C, --exclude-extension don't repack tables which belong to specific extension\n");
printf(" -X, --exclude-table don't repack specific table\n");
printf(" -Y, --exclude-parent-table don't repack specific parent table and its inheritors\n");
printf(" --error-on-invalid-index don't repack tables which belong to specific extension\n");
printf(" --switch-threshold switch tables when that many tuples are left to catchup\n");
printf(" --switch-threshold switch tables when that many tuples are left to catchup\n");
}
20 changes: 20 additions & 0 deletions doc/pg_repack.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ Options:
-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
-X, --exclude-table don't repack specific table
-Y, --exclude-parent-table don't repack specific parent table and its inheritors
--error-on-invalid-index don't repack when invalid index is found
--switch-threshold switch tables when that many tuples are left to catchup

Expand Down Expand Up @@ -226,6 +228,14 @@ Reorg Options
Skip tables that belong to the specified extension(s). Some extensions
may heavily depend on such tables at planning time etc.

``-X``, ``--exclude-table``
Skip the specified table(s) only to repack. Multiple tables may be skipped by
writing multiple ``-X`` switches.

``-Y``, ``--exclude-parent-table``
Skip the specified parent table(s) and its inheritors to repack. Multiple tables
may be skipped by writing multiple ``-Y`` switches.

``--switch-threshold``
Switch tables when that many tuples are left in log table.
This setting can be used to avoid the inability to catchup with write-heavy tables.
Expand Down Expand Up @@ -326,6 +336,16 @@ Move the specified index to tablespace ``tbs``::

$ pg_repack -d test --index idx --tablespace tbs

Skip the specified table(s) to repack ``foo`` in schema ``schema`` in
the database ``test``::

$ pg_repack -d test -X schema.foo

Skip the specified parent table(s) and its inheritors to repack ``foo`` in
schema ``schema`` in the database ``test``::

$ pg_repack -d test -Y schema.foo


Diagnostics
-----------
Expand Down