Re: Add --tablespace option to reindexdb

> On Mar 1, 2021, at 10:04 PM, Michael Paquier <michael@paquier.xyz> wrote:
> 
> On Mon, Mar 01, 2021 at 10:12:54AM -0800, Mark Dilger wrote:
>> Your check verifies that reindexing a system table on a new
>> tablespace fails, but does not verify what the failure was.  I
>> wonder if you might want to make it more robust, something like:
> 
> I was not sure if that was worth adding or not, but no objections to
> do so.  So updated the patch to do that.
> 
> On Mon, Mar 01, 2021 at 09:47:57AM -0800, Mark Dilger wrote:
>> 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.
> 
> Okay, I have made the variable names more explicit.
> 
>> 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?
> 
> While hacking on this feature I have just bumped into this separate
> issue, where the same test just gets repeated twice.  I have just
> applied an independent patch to take care of this problem separately,
> and backpatched it down to 12 where this got introduced.
> 
> On Mon, Mar 01, 2021 at 09:26:03AM -0800, Mark Dilger wrote:
>> I think the language "to reindex on" could lead users to think that
>> indexes already existent in the given tablespace will be reindexed.
>> In other words, the option might be misconstrued as a way to specify
>> all the indexes you want reindexed.  Whatever you do here, beware
>> that you are using similar language in the --help, so consider
>> changing that, too.
> 
> OK.  I have switched the docs to "Specifies the tablespace where
> indexes are rebuilt" and --help to "tablespace where indexes are
> rebuilt".
> 
> I have also removed the assertions based on the version number of the
> backend, based on Daniel's input sent upthread.
> 
> What do you think?

Looks good.

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