From 24c928ad9ad801048684569d2b67463c3ec8fdb0 Mon Sep 17 00:00:00 2001 From: Nathan Bossart Date: Mon, 11 Mar 2024 15:42:27 -0500 Subject: [PATCH] reindexdb: Allow specifying objects to process in all databases. Presently, reindexdb's --table, --schema, --index, and --system options cannot be used together with --all, i.e., you cannot specify objects to process in all databases. This commit removes this unnecessary restriction. Furthermore, it removes the restriction that --system cannot be used with --table, --schema, and --index. There is no such restriction for the latter options, and there is no technical reason to disallow these combinations. Reviewed-by: Kyotaro Horiguchi, Dean Rasheed Discussion: https://postgr.es/m/20230628232402.GA1954626%40nathanxps13 --- doc/src/sgml/ref/reindexdb.sgml | 38 ++++---- src/bin/scripts/reindexdb.c | 120 +++++++++++++------------ src/bin/scripts/t/090_reindexdb.pl | 14 +++ src/bin/scripts/t/091_reindexdb_all.pl | 20 +++++ 4 files changed, 113 insertions(+), 79 deletions(-) diff --git a/doc/src/sgml/ref/reindexdb.sgml b/doc/src/sgml/ref/reindexdb.sgml index 8d9ced212f..a877439dc5 100644 --- a/doc/src/sgml/ref/reindexdb.sgml +++ b/doc/src/sgml/ref/reindexdb.sgml @@ -55,30 +55,22 @@ PostgreSQL documentation - dbname - + + + + + + + + - - reindexdb - connection-option - option - - - - - - - - - reindexdb - connection-option - option - - - - - - dbname + + + dbname + + + + diff --git a/src/bin/scripts/reindexdb.c b/src/bin/scripts/reindexdb.c index 6ae30dff31..231e5c8fd0 100644 --- a/src/bin/scripts/reindexdb.c +++ b/src/bin/scripts/reindexdb.c @@ -46,7 +46,10 @@ static void reindex_one_database(ConnParams *cparams, ReindexType type, static void reindex_all_databases(ConnParams *cparams, const char *progname, bool echo, bool quiet, bool verbose, bool concurrently, - int concurrentCons, const char *tablespace); + int concurrentCons, const char *tablespace, + bool syscatalog, SimpleStringList *schemas, + SimpleStringList *tables, + SimpleStringList *indexes); static void run_reindex_command(PGconn *conn, ReindexType type, const char *name, bool echo, bool verbose, bool concurrently, bool async, @@ -203,62 +206,33 @@ main(int argc, char *argv[]) setup_cancel_handler(NULL); - if (alldb) - { - if (dbname) - pg_fatal("cannot reindex all databases and a specific one at the same time"); - if (syscatalog) - pg_fatal("cannot reindex all databases and system catalogs at the same time"); - if (schemas.head != NULL) - pg_fatal("cannot reindex specific schema(s) in all databases"); - if (tables.head != NULL) - pg_fatal("cannot reindex specific table(s) in all databases"); - if (indexes.head != NULL) - pg_fatal("cannot reindex specific index(es) in all databases"); - - cparams.dbname = maintenance_db; - - reindex_all_databases(&cparams, progname, echo, quiet, verbose, - concurrently, concurrentCons, tablespace); - } - else if (syscatalog) - { - if (schemas.head != NULL) - pg_fatal("cannot reindex specific schema(s) and system catalogs at the same time"); - if (tables.head != NULL) - pg_fatal("cannot reindex specific table(s) and system catalogs at the same time"); - if (indexes.head != NULL) - pg_fatal("cannot reindex specific index(es) and system catalogs at the same time"); - - if (concurrentCons > 1) - pg_fatal("cannot use multiple jobs to reindex system catalogs"); - - if (dbname == NULL) - { - if (getenv("PGDATABASE")) - dbname = getenv("PGDATABASE"); - else if (getenv("PGUSER")) - dbname = getenv("PGUSER"); - else - dbname = get_user_name_or_exit(progname); - } - - cparams.dbname = dbname; - - reindex_one_database(&cparams, REINDEX_SYSTEM, NULL, - progname, echo, verbose, - concurrently, 1, tablespace); - } - else + if (concurrentCons > 1) { /* * Index-level REINDEX is not supported with multiple jobs as we * cannot control the concurrent processing of multiple indexes * depending on the same relation. */ - if (concurrentCons > 1 && indexes.head != NULL) + if (indexes.head != NULL) pg_fatal("cannot use multiple jobs to reindex indexes"); + if (syscatalog) + pg_fatal("cannot use multiple jobs to reindex system catalogs"); + } + + if (alldb) + { + if (dbname) + pg_fatal("cannot reindex all databases and a specific one at the same time"); + + cparams.dbname = maintenance_db; + + reindex_all_databases(&cparams, progname, echo, quiet, verbose, + concurrently, concurrentCons, tablespace, + syscatalog, &schemas, &tables, &indexes); + } + else + { if (dbname == NULL) { if (getenv("PGDATABASE")) @@ -271,6 +245,11 @@ main(int argc, char *argv[]) cparams.dbname = dbname; + if (syscatalog) + reindex_one_database(&cparams, REINDEX_SYSTEM, NULL, + progname, echo, verbose, + concurrently, 1, tablespace); + if (schemas.head != NULL) reindex_one_database(&cparams, REINDEX_SCHEMA, &schemas, progname, echo, verbose, @@ -287,10 +266,11 @@ main(int argc, char *argv[]) concurrently, concurrentCons, tablespace); /* - * reindex database only if neither index nor table nor schema is - * specified + * reindex database only if neither index nor table nor schema nor + * system catalogs is specified */ - if (indexes.head == NULL && tables.head == NULL && schemas.head == NULL) + if (!syscatalog && indexes.head == NULL && + tables.head == NULL && schemas.head == NULL) reindex_one_database(&cparams, REINDEX_DATABASE, NULL, progname, echo, verbose, concurrently, concurrentCons, tablespace); @@ -711,7 +691,9 @@ static void reindex_all_databases(ConnParams *cparams, const char *progname, bool echo, bool quiet, bool verbose, bool concurrently, int concurrentCons, - const char *tablespace) + const char *tablespace, bool syscatalog, + SimpleStringList *schemas, SimpleStringList *tables, + SimpleStringList *indexes) { PGconn *conn; PGresult *result; @@ -735,9 +717,35 @@ reindex_all_databases(ConnParams *cparams, cparams->override_dbname = dbname; - reindex_one_database(cparams, REINDEX_DATABASE, NULL, - progname, echo, verbose, concurrently, - concurrentCons, tablespace); + if (syscatalog) + reindex_one_database(cparams, REINDEX_SYSTEM, NULL, + progname, echo, verbose, + concurrently, 1, tablespace); + + if (schemas->head != NULL) + reindex_one_database(cparams, REINDEX_SCHEMA, schemas, + progname, echo, verbose, + concurrently, concurrentCons, tablespace); + + if (indexes->head != NULL) + reindex_one_database(cparams, REINDEX_INDEX, indexes, + progname, echo, verbose, + concurrently, 1, tablespace); + + if (tables->head != NULL) + reindex_one_database(cparams, REINDEX_TABLE, tables, + progname, echo, verbose, + concurrently, concurrentCons, tablespace); + + /* + * reindex database only if neither index nor table nor schema nor + * system catalogs is specified + */ + if (!syscatalog && indexes->head == NULL && + tables->head == NULL && schemas->head == NULL) + reindex_one_database(cparams, REINDEX_DATABASE, NULL, + progname, echo, verbose, + concurrently, concurrentCons, tablespace); } PQclear(result); diff --git a/src/bin/scripts/t/090_reindexdb.pl b/src/bin/scripts/t/090_reindexdb.pl index 4f1a141132..429dd3acd6 100644 --- a/src/bin/scripts/t/090_reindexdb.pl +++ b/src/bin/scripts/t/090_reindexdb.pl @@ -262,4 +262,18 @@ $node->command_ok( [ 'reindexdb', '-j', '2', '--concurrently', '-d', 'postgres' ], 'parallel reindexdb on database, concurrently'); +# combinations of objects +$node->issues_sql_like( + [ 'reindexdb', '-s', '-t', 'test1', 'postgres' ], + qr/statement:\ REINDEX SYSTEM postgres;/, + 'specify both --system and --table'); +$node->issues_sql_like( + [ 'reindexdb', '-s', '-i', 'test1x', 'postgres' ], + qr/statement:\ REINDEX INDEX public.test1x;/, + 'specify both --system and --index'); +$node->issues_sql_like( + [ 'reindexdb', '-s', '-S', 'pg_catalog', 'postgres' ], + qr/statement:\ REINDEX SCHEMA pg_catalog;/, + 'specify both --system and --schema'); + done_testing(); diff --git a/src/bin/scripts/t/091_reindexdb_all.pl b/src/bin/scripts/t/091_reindexdb_all.pl index a4540084fe..8061883f7f 100644 --- a/src/bin/scripts/t/091_reindexdb_all.pl +++ b/src/bin/scripts/t/091_reindexdb_all.pl @@ -13,10 +13,30 @@ $node->start; $ENV{PGOPTIONS} = '--client-min-messages=WARNING'; +$node->safe_psql('postgres', + 'CREATE TABLE test1 (a int); CREATE INDEX test1x ON test1 (a);'); +$node->safe_psql('template1', + 'CREATE TABLE test1 (a int); CREATE INDEX test1x ON test1 (a);'); $node->issues_sql_like( [ 'reindexdb', '-a' ], qr/statement: REINDEX.*statement: REINDEX/s, 'reindex all databases'); +$node->issues_sql_like( + [ 'reindexdb', '-a', '-s' ], + qr/statement: REINDEX SYSTEM postgres/s, + 'reindex system catalogs in all databases'); +$node->issues_sql_like( + [ 'reindexdb', '-a', '-S', 'public' ], + qr/statement: REINDEX SCHEMA public/s, + 'reindex schema in all databases'); +$node->issues_sql_like( + [ 'reindexdb', '-a', '-i', 'test1x' ], + qr/statement: REINDEX INDEX public\.test1x/s, + 'reindex index in all databases'); +$node->issues_sql_like( + [ 'reindexdb', '-a', '-t', 'test1' ], + qr/statement: REINDEX TABLE public\.test1/s, + 'reindex table in all databases'); $node->safe_psql( 'postgres', q(