Re: Add --tablespace option to reindexdb

> On Feb 25, 2021, at 10:49 PM, Michael Paquier <michael@paquier.xyz> wrote:
> 
> While on it, I have added tests for toast tables and indexes with a
> tablespace move during a REINDEX. 


In t/090_reindexdb.pl, you add:

+# Create a tablespace for testing.
+my $ts = $node->basedir . '/regress_reindex_tbspace';
+mkdir $ts or die "cannot create directory $ts";
+# this takes care of WIN-specific path issues
+my $ets     = TestLib::perl2host($ts);
+my $tbspace = 'reindex_tbspace';
+$node->safe_psql('postgres', "CREATE TABLESPACE $tbspace LOCATION '$ets';");

I recognize that you are borrowing some of that from src/bin/pgbench/t/001_pgbench_with_server.pl, but I don't find anything intuitive about the name "ets" and would rather not see that repeated.  There is nothing in TestLib::perl2host to explain this name choice, nor in pgbench's test, nor yours.

You also change a test:

 $node->issues_sql_like(
-       [ 'reindexdb', '-v', '-t', 'test1', 'postgres' ],
-       qr/statement: REINDEX \(VERBOSE\) TABLE public\.test1;/,
-       'reindex with verbose output');
+       [ 'reindexdb', '--concurrently', '-v', '-t', 'test1', 'postgres' ],
+       qr/statement: REINDEX \(VERBOSE\) TABLE CONCURRENTLY public\.test1;/,
+       'reindex concurrently with verbose output');

but I don't see what tests of the --concurrently option have to do with this patch.  I'm not saying there is anything wrong with this change, but it seems out of place.  Am I missing something?

—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company