Re: Add --tablespace option to reindexdb

> On Feb 25, 2021, at 10:49 PM, Michael Paquier <michael(at)paquier(dot)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