> On Mar 15, 2021, at 10:50 AM, Julien Rouhaud <rjuju...@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
