Re: Add --tablespace option to reindexdb

Michael Paquier <michael@paquier.xyz>

From: Michael Paquier <michael@paquier.xyz>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Postgres hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-03-02T06:04:58Z
Lists: pgsql-hackers

Attachments

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?
--
Michael

Commits

  1. Add --tablespace option to reindexdb

  2. Fix duplicated test case in TAP tests of reindexdb