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
- v2-0001-Add-tablespace-option-to-reindexdb.patch (text/x-diff) patch v2-0001
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
-
Add --tablespace option to reindexdb
- 57e6db706e81 14.0 landed
-
Fix duplicated test case in TAP tests of reindexdb
- 0f1b0c0b60ef 12.7 landed
- 621e8a603d56 13.3 landed
- c5530d8474a4 14.0 landed