> On Feb 25, 2021, at 10:49 PM, Michael Paquier <mich...@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