Re: REINDEX backend filtering

> On Mar 15, 2021, at 10:50 AM, Julien Rouhaud <rjuju123(at)gmail(dot)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