Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2024-03-11 Thread Nathan Bossart
On Fri, Mar 08, 2024 at 04:03:22PM -0600, Nathan Bossart wrote:
> On Fri, Mar 08, 2024 at 09:33:19AM +, Dean Rasheed wrote:
>> I think this is good to go.
> 
> Thanks.  In v4, I've added a first draft of the commit messages, and I am
> planning to commit this early next week.

Committed.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2024-03-08 Thread Nathan Bossart
On Fri, Mar 08, 2024 at 09:33:19AM +, Dean Rasheed wrote:
> If I'm nitpicking, "[--verbose | -v]" in the clusterdb synopsis should
> be replaced with "[option...]", like the other commands, because there
> are other general-purpose options like --quiet and --echo.

Good catch.  I fixed that in v4.  We could probably back-patch this
particular change, but since it's been this way for a while, I don't think
it's terribly important to do so.

> I think this is good to go.

Thanks.  In v4, I've added a first draft of the commit messages, and I am
planning to commit this early next week.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 460da2161265b042079727c1178eff92b3d537b6 Mon Sep 17 00:00:00 2001
From: Nathan Bossart 
Date: Fri, 8 Mar 2024 14:35:07 -0600
Subject: [PATCH v4 1/3] vacuumdb: Allow specifying objects to process in all
 databases.

Presently, vacuumdb's --table, --schema, and --exclude-schema
options cannot be used together with --all, i.e., you cannot
specify tables or schemas to process in all databases.  This commit
removes this unnecessary restriction, thus enabling potentially
useful command like "vacuumdb --all --schema pg_catalog".

Reviewed-by: Kyotaro Horiguchi, Dean Rasheed
Discussion: https://postgr.es/m/20230628232402.GA1954626%40nathanxps13
---
 doc/src/sgml/ref/vacuumdb.sgml| 60 ++-
 src/bin/scripts/t/100_vacuumdb.pl | 24 ++---
 src/bin/scripts/vacuumdb.c| 19 +++---
 3 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 09356ea4fa..66fccb30a2 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -36,7 +36,13 @@ PostgreSQL documentation
 

 
-   dbname
+   
+
+ dbname
+ -a
+ --all
+
+   
   
 
   
@@ -47,40 +53,44 @@ PostgreSQL documentation

 
  
-   
-
- 
-  -n
-  --schema
- 
- schema
-
-   
-
-   
-
- 
-  -N
-  --exclude-schema
- 
- schema
-
-   
+  -n
+  --schema
  
+ schema
 

 
-   dbname
+   
+
+ dbname
+ -a
+ --all
+
+   
   
 
   
vacuumdb
connection-option
option
-   
--a
---all
-   
+
+   
+
+ 
+  -N
+  --exclude-schema
+ 
+ schema
+
+   
+
+   
+
+ dbname
+ -a
+ --all
+
+   
   
  
 
diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 0601fde205..1d8558c780 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -184,18 +184,18 @@ $node->command_fails_like(
 	[ 'vacuumdb', '-n', 'pg_catalog', '-N', '"Foo"', 'postgres' ],
 	qr/cannot vacuum all tables in schema\(s\) and exclude schema\(s\) at the same time/,
 	'cannot use options -n and -N at the same time');
-$node->command_fails_like(
-	[ 'vacuumdb', '-a', '-N', '"Foo"' ],
-	qr/cannot exclude specific schema\(s\) in all databases/,
-	'cannot use options -a and -N at the same time');
-$node->command_fails_like(
-	[ 'vacuumdb', '-a', '-n', '"Foo"' ],
-	qr/cannot vacuum specific schema\(s\) in all databases/,
-	'cannot use options -a and -n at the same time');
-$node->command_fails_like(
-	[ 'vacuumdb', '-a', '-t', '"Foo".bar' ],
-	qr/cannot vacuum specific table\(s\) in all databases/,
-	'cannot use options -a and -t at the same time');
+$node->issues_sql_like(
+	[ 'vacuumdb', '-a', '-N', 'pg_catalog' ],
+	qr/(?:(?!VACUUM \(SKIP_DATABASE_STATS\) pg_catalog.pg_class).)*/,
+	'vacuumdb -a -N');
+$node->issues_sql_like(
+	[ 'vacuumdb', '-a', '-n', 'pg_catalog' ],
+	qr/VACUUM \(SKIP_DATABASE_STATS\) pg_catalog.pg_class/,
+	'vacuumdb -a -n');
+$node->issues_sql_like(
+	[ 'vacuumdb', '-a', '-t', 'pg_class' ],
+	qr/VACUUM \(SKIP_DATABASE_STATS\) pg_catalog.pg_class/,
+	'vacuumdb -a -t');
 $node->command_fails_like(
 	[ 'vacuumdb', '-a', '-d', 'postgres' ],
 	qr/cannot vacuum all databases and a specific one at the same time/,
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 291766793e..7138c6e97e 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -72,6 +72,7 @@ static void vacuum_one_database(ConnParams *cparams,
 static void vacuum_all_databases(ConnParams *cparams,
  vacuumingOptions *vacopts,
  bool analyze_in_stages,
+ SimpleStringList *objects,
  int concurrentCons,
  const char *progname, bool echo, bool quiet);
 
@@ -378,6 +379,7 @@ main(int argc, char *argv[])
 
 		vacuum_all_databases(, ,
 			 analyze_in_stages,
+			 ,
 			 concurrentCons,
 			 progname, echo, quiet);
 	}
@@ -429,18 +431,6 @@ check_objfilter(void)
 		(objfilter & OBJFILTER_DATABASE))
 		pg_fatal("cannot vacuum all databases and a specific one at the same time");
 
-	if ((objfilter & OBJFILTER_ALL_DBS) &&
-		(objfilter & 

Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2024-03-08 Thread Dean Rasheed
On Wed, 6 Mar 2024 at 22:22, Nathan Bossart  wrote:
>
> Thanks for taking a look.  I updated the synopsis sections in v3.

OK, that looks good. The vacuumdb synopsis in particular looks a lot
better now that "-N | --exclude-schema" is on its own line, because it
was hard to read previously, and easy to mistakenly think that -n
could be combined with -N.

If I'm nitpicking, "[--verbose | -v]" in the clusterdb synopsis should
be replaced with "[option...]", like the other commands, because there
are other general-purpose options like --quiet and --echo.

> I also spent some more time on the reindexdb patch (0003).  I previously
> had decided to restrict combinations of tables, schemas, and indexes
> because I felt it was "ambiguous and inconsistent with vacuumdb," but
> looking closer, I think that's the wrong move.  reindexdb already supports
> such combinations, which it interprets to mean it should reindex each
> listed object.  So, I removed that change in v3.

Makes sense.

> Even though reindexdb allows combinations of tables, schema, and indexes,
> it doesn't allow combinations of "system catalogs" and other objects, and
> it's not clear why.  In v3, I've removed this restriction, which ended up
> simplifying the 0003 patch a bit.  Like combinations of tables, schemas,
> and indexes, reindexdb will now interpret combinations that include
> --system to mean it should reindex each listed object as well as the system
> catalogs.

OK, that looks useful, especially given that most people will still
probably use this against a single database, and it's making that more
flexible.

I think this is good to go.

Regards,
Dean




Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2024-03-06 Thread Nathan Bossart
On Tue, Mar 05, 2024 at 11:20:13PM +, Dean Rasheed wrote:
> I'm not sure how useful these changes are, but I don't really object.
> You need to update the synopsis section of the docs though.

Thanks for taking a look.  I updated the synopsis sections in v3.

I also spent some more time on the reindexdb patch (0003).  I previously
had decided to restrict combinations of tables, schemas, and indexes
because I felt it was "ambiguous and inconsistent with vacuumdb," but
looking closer, I think that's the wrong move.  reindexdb already supports
such combinations, which it interprets to mean it should reindex each
listed object.  So, I removed that change in v3.

Even though reindexdb allows combinations of tables, schema, and indexes,
it doesn't allow combinations of "system catalogs" and other objects, and
it's not clear why.  In v3, I've removed this restriction, which ended up
simplifying the 0003 patch a bit.  Like combinations of tables, schemas,
and indexes, reindexdb will now interpret combinations that include
--system to mean it should reindex each listed object as well as the system
catalogs.

Ideally, we'd allow similar combinations in vacuumdb, but I believe that
would require a much more invasive patch, and I've already spent far more
time on this change than I wanted to.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 84b3f5a8275d53707b15208d761567372b7b20a5 Mon Sep 17 00:00:00 2001
From: Nathan Bossart 
Date: Wed, 28 Jun 2023 15:12:18 -0700
Subject: [PATCH v3 1/3] vacuumdb: allow specifying tables or schemas to
 process in all databases

---
 doc/src/sgml/ref/vacuumdb.sgml| 60 ++-
 src/bin/scripts/t/100_vacuumdb.pl | 24 ++---
 src/bin/scripts/vacuumdb.c| 19 +++---
 3 files changed, 52 insertions(+), 51 deletions(-)

diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 09356ea4fa..66fccb30a2 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -36,7 +36,13 @@ PostgreSQL documentation
 

 
-   dbname
+   
+
+ dbname
+ -a
+ --all
+
+   
   
 
   
@@ -47,40 +53,44 @@ PostgreSQL documentation

 
  
-   
-
- 
-  -n
-  --schema
- 
- schema
-
-   
-
-   
-
- 
-  -N
-  --exclude-schema
- 
- schema
-
-   
+  -n
+  --schema
  
+ schema
 

 
-   dbname
+   
+
+ dbname
+ -a
+ --all
+
+   
   
 
   
vacuumdb
connection-option
option
-   
--a
---all
-   
+
+   
+
+ 
+  -N
+  --exclude-schema
+ 
+ schema
+
+   
+
+   
+
+ dbname
+ -a
+ --all
+
+   
   
  
 
diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 0601fde205..1d8558c780 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -184,18 +184,18 @@ $node->command_fails_like(
 	[ 'vacuumdb', '-n', 'pg_catalog', '-N', '"Foo"', 'postgres' ],
 	qr/cannot vacuum all tables in schema\(s\) and exclude schema\(s\) at the same time/,
 	'cannot use options -n and -N at the same time');
-$node->command_fails_like(
-	[ 'vacuumdb', '-a', '-N', '"Foo"' ],
-	qr/cannot exclude specific schema\(s\) in all databases/,
-	'cannot use options -a and -N at the same time');
-$node->command_fails_like(
-	[ 'vacuumdb', '-a', '-n', '"Foo"' ],
-	qr/cannot vacuum specific schema\(s\) in all databases/,
-	'cannot use options -a and -n at the same time');
-$node->command_fails_like(
-	[ 'vacuumdb', '-a', '-t', '"Foo".bar' ],
-	qr/cannot vacuum specific table\(s\) in all databases/,
-	'cannot use options -a and -t at the same time');
+$node->issues_sql_like(
+	[ 'vacuumdb', '-a', '-N', 'pg_catalog' ],
+	qr/(?:(?!VACUUM \(SKIP_DATABASE_STATS\) pg_catalog.pg_class).)*/,
+	'vacuumdb -a -N');
+$node->issues_sql_like(
+	[ 'vacuumdb', '-a', '-n', 'pg_catalog' ],
+	qr/VACUUM \(SKIP_DATABASE_STATS\) pg_catalog.pg_class/,
+	'vacuumdb -a -n');
+$node->issues_sql_like(
+	[ 'vacuumdb', '-a', '-t', 'pg_class' ],
+	qr/VACUUM \(SKIP_DATABASE_STATS\) pg_catalog.pg_class/,
+	'vacuumdb -a -t');
 $node->command_fails_like(
 	[ 'vacuumdb', '-a', '-d', 'postgres' ],
 	qr/cannot vacuum all databases and a specific one at the same time/,
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 291766793e..7138c6e97e 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -72,6 +72,7 @@ static void vacuum_one_database(ConnParams *cparams,
 static void vacuum_all_databases(ConnParams *cparams,
  vacuumingOptions *vacopts,
  bool analyze_in_stages,
+ SimpleStringList *objects,
  int concurrentCons,
  const char *progname, bool echo, bool quiet);
 
@@ -378,6 +379,7 @@ main(int argc, char *argv[])
 
 		vacuum_all_databases(, ,
 			 analyze_in_stages,

Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2024-03-05 Thread Dean Rasheed
On Tue, 5 Mar 2024 at 02:22, Nathan Bossart  wrote:
>
> I saw that this thread was referenced elsewhere [0], so I figured I'd take
> a fresh look.  From a quick glance, I'd say 0001 and 0002 are pretty
> reasonable and could probably be committed for v17.
>

I'm not sure how useful these changes are, but I don't really object.
You need to update the synopsis section of the docs though.

Regards,
Dean




Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2024-03-04 Thread Nathan Bossart
On Mon, Oct 23, 2023 at 03:25:42PM -0500, Nathan Bossart wrote:
> rebased

I saw that this thread was referenced elsewhere [0], so I figured I'd take
a fresh look.  From a quick glance, I'd say 0001 and 0002 are pretty
reasonable and could probably be committed for v17.  0003 probably requires
some more attention.  If there is indeed interest in these changes, I'll
try to spend some more time on it.

[0] https://postgr.es/m/E0D2F0CE-D27C-49B1-902B-AD8D2427F07E%40yandex-team.ru

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2023-10-23 Thread Nathan Bossart
rebased

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com
>From 9618c243cbd3056006acda0136036b432af37830 Mon Sep 17 00:00:00 2001
From: Nathan Bossart 
Date: Wed, 28 Jun 2023 15:12:18 -0700
Subject: [PATCH v2 1/3] vacuumdb: allow specifying tables or schemas to
 process in all databases

---
 src/bin/scripts/t/100_vacuumdb.pl | 24 
 src/bin/scripts/vacuumdb.c| 19 +--
 2 files changed, 17 insertions(+), 26 deletions(-)

diff --git a/src/bin/scripts/t/100_vacuumdb.pl b/src/bin/scripts/t/100_vacuumdb.pl
index 925079bbed..52926d53a5 100644
--- a/src/bin/scripts/t/100_vacuumdb.pl
+++ b/src/bin/scripts/t/100_vacuumdb.pl
@@ -188,18 +188,18 @@ $node->command_fails_like(
 	[ 'vacuumdb', '-n', 'pg_catalog', '-N', '"Foo"', 'postgres' ],
 	qr/cannot vacuum all tables in schema\(s\) and exclude schema\(s\) at the same time/,
 	'cannot use options -n and -N at the same time');
-$node->command_fails_like(
-	[ 'vacuumdb', '-a', '-N', '"Foo"' ],
-	qr/cannot exclude specific schema\(s\) in all databases/,
-	'cannot use options -a and -N at the same time');
-$node->command_fails_like(
-	[ 'vacuumdb', '-a', '-n', '"Foo"' ],
-	qr/cannot vacuum specific schema\(s\) in all databases/,
-	'cannot use options -a and -n at the same time');
-$node->command_fails_like(
-	[ 'vacuumdb', '-a', '-t', '"Foo".bar' ],
-	qr/cannot vacuum specific table\(s\) in all databases/,
-	'cannot use options -a and -t at the same time');
+$node->issues_sql_like(
+	[ 'vacuumdb', '-a', '-N', 'pg_catalog' ],
+	qr/(?:(?!VACUUM \(SKIP_DATABASE_STATS\) pg_catalog.pg_class).)*/,
+	'vacuumdb -a -N');
+$node->issues_sql_like(
+	[ 'vacuumdb', '-a', '-n', 'pg_catalog' ],
+	qr/VACUUM \(SKIP_DATABASE_STATS\) pg_catalog.pg_class/,
+	'vacuumdb -a -n');
+$node->issues_sql_like(
+	[ 'vacuumdb', '-a', '-t', 'pg_class' ],
+	qr/VACUUM \(SKIP_DATABASE_STATS\) pg_catalog.pg_class/,
+	'vacuumdb -a -t');
 $node->command_fails_like(
 	[ 'vacuumdb', '-a', '-d', 'postgres' ],
 	qr/cannot vacuum all databases and a specific one at the same time/,
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index d682573dc1..0eddcaa047 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -72,6 +72,7 @@ static void vacuum_one_database(ConnParams *cparams,
 static void vacuum_all_databases(ConnParams *cparams,
  vacuumingOptions *vacopts,
  bool analyze_in_stages,
+ SimpleStringList *objects,
  int concurrentCons,
  const char *progname, bool echo, bool quiet);
 
@@ -378,6 +379,7 @@ main(int argc, char *argv[])
 
 		vacuum_all_databases(, ,
 			 analyze_in_stages,
+			 ,
 			 concurrentCons,
 			 progname, echo, quiet);
 	}
@@ -429,18 +431,6 @@ check_objfilter(void)
 		(objfilter & OBJFILTER_DATABASE))
 		pg_fatal("cannot vacuum all databases and a specific one at the same time");
 
-	if ((objfilter & OBJFILTER_ALL_DBS) &&
-		(objfilter & OBJFILTER_TABLE))
-		pg_fatal("cannot vacuum specific table(s) in all databases");
-
-	if ((objfilter & OBJFILTER_ALL_DBS) &&
-		(objfilter & OBJFILTER_SCHEMA))
-		pg_fatal("cannot vacuum specific schema(s) in all databases");
-
-	if ((objfilter & OBJFILTER_ALL_DBS) &&
-		(objfilter & OBJFILTER_SCHEMA_EXCLUDE))
-		pg_fatal("cannot exclude specific schema(s) in all databases");
-
 	if ((objfilter & OBJFILTER_TABLE) &&
 		(objfilter & OBJFILTER_SCHEMA))
 		pg_fatal("cannot vacuum all tables in schema(s) and specific table(s) at the same time");
@@ -895,6 +885,7 @@ static void
 vacuum_all_databases(ConnParams *cparams,
 	 vacuumingOptions *vacopts,
 	 bool analyze_in_stages,
+	 SimpleStringList *objects,
 	 int concurrentCons,
 	 const char *progname, bool echo, bool quiet)
 {
@@ -927,7 +918,7 @@ vacuum_all_databases(ConnParams *cparams,
 
 vacuum_one_database(cparams, vacopts,
 	stage,
-	NULL,
+	objects,
 	concurrentCons,
 	progname, echo, quiet);
 			}
@@ -941,7 +932,7 @@ vacuum_all_databases(ConnParams *cparams,
 
 			vacuum_one_database(cparams, vacopts,
 ANALYZE_NO_STAGE,
-NULL,
+objects,
 concurrentCons,
 progname, echo, quiet);
 		}
-- 
2.25.1

>From de22c8c0060512d1a9add03b6eb4265767fb061b Mon Sep 17 00:00:00 2001
From: Nathan Bossart 
Date: Wed, 28 Jun 2023 15:12:58 -0700
Subject: [PATCH v2 2/3] clusterdb: allow specifying tables to process in all
 databases

---
 src/bin/scripts/clusterdb.c| 28 +-
 src/bin/scripts/t/011_clusterdb_all.pl | 11 ++
 2 files changed, 30 insertions(+), 9 deletions(-)

diff --git a/src/bin/scripts/clusterdb.c b/src/bin/scripts/clusterdb.c
index 65428031c7..89f1e733fe 100644
--- a/src/bin/scripts/clusterdb.c
+++ b/src/bin/scripts/clusterdb.c
@@ -21,8 +21,9 @@
 
 static void cluster_one_database(const ConnParams *cparams, const char *table,
  const char *progname, bool verbose, bool echo);
-static void 

Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2023-06-29 Thread Nathan Bossart
On Fri, Jun 30, 2023 at 12:05:17PM +0900, Kyotaro Horiguchi wrote:
> At Thu, 29 Jun 2023 13:56:38 -0700, Nathan Bossart  
> wrote in 
>> Sorry, I'm not following.  I intentionally avoided allowing combinations of
>> --schema and --table in the patches I sent.  This is the current behavior
>> of vacuumdb.  Are you suggesting that they should be treated as restriction
>> filters?
> 
> No. I'm not suggesting. Just saying that they would look appear to
> work as a restriction filters if those two options can be specified at
> once.

Got it, thanks for clarifying.

>> Perhaps we could add something like a --skip-missing option.
> 
> But isn't it a bit too complicated for the gain?
> 
> I don't have a strong objection if we're fine with just allowing
> "--all --schema=xxx", knowing that it will works cleanly only for
> system catalogs.

Okay.  I haven't scoped out what would be required to support a
--skip-missing option, but it doesn't sound too terribly complicated to me.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2023-06-29 Thread Kyotaro Horiguchi
At Thu, 29 Jun 2023 13:56:38 -0700, Nathan Bossart  
wrote in 
> Thanks for taking a look.
> 
> On Thu, Jun 29, 2023 at 02:16:26PM +0900, Kyotaro Horiguchi wrote:
> > At Wed, 28 Jun 2023 16:24:02 -0700, Nathan Bossart 
> >  wrote in 
> >> I debated also allowing users to specify different types of objects in the
> >> same command (e.g., "vacuumdb --schema myschema --table mytable"), but it
> >> looked like this would require a more substantial rewrite, and I didn't
> >> feel that the behavior was intuitive.  For the example I just gave, does
> >> the user expect us to process both the "myschema" schema and the "mytable"
> >> table, or does the user want us to process the "mytable" table in the
> >> "myschema" schema?  In vacuumdb, this is already blocked, but reindexdb
> > 
> > I think spcyfying the two at once is inconsistent if we maintain the
> > current behavior of those options.
> > 
> > It seems to me that that change clearly modifies the functionality of
> > the options. As a result, those options look like restriction
> > filters. For example, "vacuumdb -s s1_* -t t1" will vacuum all table
> > named "t1" in all schemas matches "s1_*".
> 
> Sorry, I'm not following.  I intentionally avoided allowing combinations of
> --schema and --table in the patches I sent.  This is the current behavior
> of vacuumdb.  Are you suggesting that they should be treated as restriction
> filters?

No. I'm not suggesting. Just saying that they would look appear to
work as a restriction filters if those two options can be specified at
once.

> > While I think this is useful, primarily for system catalogs, I'm not
> > entirely convinced about its practicality to user objects. It's
> > difficult for me to imagine that a situation where all databases share
> > the same schema would be major.
> > 
> > Assuming this is used for user objects, it may be necessary to safely
> > exclude databases that lack the specified schema or table, provided
> > the object present in at least one other database. But the exclusion
> > should be done with printing some warnings.  It could also be
> > necessary to safely move to the next object when reindex or cluster
> > operation fails on a single object due to missing prerequisite
> > situations. But I don't think we might want to add such complexity to
> > these "script" tools.
> 
> Perhaps we could add something like a --skip-missing option.

But isn't it a bit too complicated for the gain?

I don't have a strong objection if we're fine with just allowing
"--all --schema=xxx", knowing that it will works cleanly only for
system catalogs.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center




Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2023-06-29 Thread Nathan Bossart
Thanks for taking a look.

On Thu, Jun 29, 2023 at 02:16:26PM +0900, Kyotaro Horiguchi wrote:
> At Wed, 28 Jun 2023 16:24:02 -0700, Nathan Bossart  
> wrote in 
>> I debated also allowing users to specify different types of objects in the
>> same command (e.g., "vacuumdb --schema myschema --table mytable"), but it
>> looked like this would require a more substantial rewrite, and I didn't
>> feel that the behavior was intuitive.  For the example I just gave, does
>> the user expect us to process both the "myschema" schema and the "mytable"
>> table, or does the user want us to process the "mytable" table in the
>> "myschema" schema?  In vacuumdb, this is already blocked, but reindexdb
> 
> I think spcyfying the two at once is inconsistent if we maintain the
> current behavior of those options.
> 
> It seems to me that that change clearly modifies the functionality of
> the options. As a result, those options look like restriction
> filters. For example, "vacuumdb -s s1_* -t t1" will vacuum all table
> named "t1" in all schemas matches "s1_*".

Sorry, I'm not following.  I intentionally avoided allowing combinations of
--schema and --table in the patches I sent.  This is the current behavior
of vacuumdb.  Are you suggesting that they should be treated as restriction
filters?

> While I think this is useful, primarily for system catalogs, I'm not
> entirely convinced about its practicality to user objects. It's
> difficult for me to imagine that a situation where all databases share
> the same schema would be major.
> 
> Assuming this is used for user objects, it may be necessary to safely
> exclude databases that lack the specified schema or table, provided
> the object present in at least one other database. But the exclusion
> should be done with printing some warnings.  It could also be
> necessary to safely move to the next object when reindex or cluster
> operation fails on a single object due to missing prerequisite
> situations. But I don't think we might want to add such complexity to
> these "script" tools.

Perhaps we could add something like a --skip-missing option.

-- 
Nathan Bossart
Amazon Web Services: https://aws.amazon.com




Re: vacuumdb/clusterdb/reindexdb: allow specifying objects to process in all databases

2023-06-28 Thread Kyotaro Horiguchi
At Wed, 28 Jun 2023 16:24:02 -0700, Nathan Bossart  
wrote in 
> While working on some other patches, I found myself wanting to use the
> following command to vacuum the catalogs in all databases in a cluster:
> 
>   vacuumdb --all --schema pg_catalog
> 
> However, this presently fails with the following error:
> 
>   cannot vacuum specific schema(s) in all databases
> 
> AFAICT there no technical reason to block this, and the resulting behavior
> feels intuitive to me, so I wrote 0001 to allow it.  0002 allows specifying
> tables to process in all databases in clusterdb, and 0003 allows specifying
> tables, indexes, schemas, or the system catalogs to process in all
> databases in reindexdb.

It seems like useful.

> I debated also allowing users to specify different types of objects in the
> same command (e.g., "vacuumdb --schema myschema --table mytable"), but it
> looked like this would require a more substantial rewrite, and I didn't
> feel that the behavior was intuitive.  For the example I just gave, does
> the user expect us to process both the "myschema" schema and the "mytable"
> table, or does the user want us to process the "mytable" table in the
> "myschema" schema?  In vacuumdb, this is already blocked, but reindexdb

I think spcyfying the two at once is inconsistent if we maintain the
current behavior of those options.

It seems to me that that change clearly modifies the functionality of
the options. As a result, those options look like restriction
filters. For example, "vacuumdb -s s1_* -t t1" will vacuum all table
named "t1" in all schemas matches "s1_*".

> accepts combinations of tables, schemas, and indexes (yet disallows
> specifying --system along with other types of objects).  Since this is
> inconsistent with vacuumdb and IMO ambiguous, I've restricted such
> combinations in 0003.
> 
> Thoughts?

While I think this is useful, primarily for system catalogs, I'm not
entirely convinced about its practicality to user objects. It's
difficult for me to imagine that a situation where all databases share
the same schema would be major.

Assuming this is used for user objects, it may be necessary to safely
exclude databases that lack the specified schema or table, provided
the object present in at least one other database. But the exclusion
should be done with printing some warnings.  It could also be
necessary to safely move to the next object when reindex or cluster
operation fails on a single object due to missing prerequisite
situations. But I don't think we might want to add such complexity to
these "script" tools.

So.. an alternative path might be to introduce a new option like
--syscatalog to specify system catalogs as the only option that can be
combined with --all. In doing so, we can leave the --table and
--schema options untouched.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center