> On Mar 1, 2021, at 10:04 PM, Michael Paquier <mich...@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
