From: Mark Dilger <mark.dilger@enterprisedb.com>
To: Julien Rouhaud <rjuju123@gmail.com>
Cc: Michael Paquier <michael@paquier.xyz>
Cc: Magnus Hagander <magnus@hagander.net>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: REINDEX backend filtering
Date: Mon, 15 Mar 2021 10:56:50 -0700
Message-ID: <F2282C75-D06D-4F9B-8AAD-66F206760B24@enterprisedb.com> (raw)
In-Reply-To: <20210315175044.txxhycqg7iaa66qy@nol>
References: <YE35Y//jLIb7gFLu@paquier.xyz>
<20210314145737.enqhi2jxszws3ywi@nol>
<YE6ikDiXyA0w19UI@paquier.xyz>
<20210315003551.enuwfghdh7qhfnb2@nol>
<20210315033320.4273frstqn7w6kh7@nol>
<0E0B73C0-EBC0-490A-9347-66A66E3338AE@enterprisedb.com>
<20210315165213.tgxwtjtnlnr2q3f5@nol>
<074D5DFD-4D47-4A84-93A5-C2DC8935E6E9@enterprisedb.com>
<20210315173401.yqgxgmbspiyfrbks@nol>
<9AC710C9-7006-43EC-93F6-7A7ABFE09E80@enterprisedb.com>
<20210315175044.txxhycqg7iaa66qy@nol>
> On Mar 15, 2021, at 10:50 AM, Julien Rouhaud <rjuju123@gmail.com> wrote:
>
> On Mon, Mar 15, 2021 at 10:40:25AM -0700, Mark Dilger wrote:
>> I'm saying that your patch seems to call down to get_collation_actual_version() via get_collation_version_for_oid() from your new function do_check_index_has_outdated_collation(), but I'm not seeing how that gets exercised.
>
> It's a little bit late here so sorry if I'm missing something.
>
> do_check_index_has_outdated_collation() is called from
> index_has_outdated_collation() which is called from
> index_has_outdated_dependency() which is called from
> RelationGetIndexListFiltered(), and that function is called when passing the
> OUTDATED option to REINDEX (and reindexdb --outdated). So this is exercised
> with added tests for both matching and non matching collation version.
Ok, fair enough. I was thinking about the case where the collation actually returns a different version number because it (the C library providing the collation) got updated, but I think you've answered already that you are not planning to test that case, only the case where pg_depend is modified to have a bogus version number.
It seems a bit odd to me that a feature intended to handle cases where collations are updated is not tested via having a collation be updated during the test. It leaves open the possibility that something differs between the test and reindexed run after real world collation updates. But that's for the committer who picks up your patch to decide, and perhaps it is unfair to make your patch depend on addressing that issue.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: mark.dilger@enterprisedb.com, rjuju123@gmail.com, michael@paquier.xyz, magnus@hagander.net, pgsql-hackers@lists.postgresql.org
Subject: Re: REINDEX backend filtering
In-Reply-To: <F2282C75-D06D-4F9B-8AAD-66F206760B24@enterprisedb.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox