On Fri, Nov 8, 2019 at 2:24 AM Thomas Munro <thomas.mu...@gmail.com> wrote: > > On Thu, Nov 7, 2019 at 10:20 PM Julien Rouhaud <rjuju...@gmail.com> wrote: > > > > I didn't do anything for the spurious warning when running a reindex, > > and kept original approach of pg_depend catalog. > > I see three options: > > 1. Change all or some of index_open(), relation_open(), > RelationIdGetRelation(), RelationBuildDesc() and > RelationInitIndexAccessInfo() to take some kind of flag so we can say > NO_COLLATION_VERSION_CHECK_PLEASE, and then have ReindexIndex() pass > that flag down when opening it for the purpose of rebuilding it. > 2. Use a global state to suppress these warnings while opening that > index. Perhaps ReindexIndex() would call RelCacheWarnings(false) > before index_open(), and use PG_FINALLY to make sure it restores > warnings with RelCacheWarnings(true). (This is a less-code-churn > bad-programming-style version of #1.) > 3. Move the place that we run the collation version check. Instead > of doing it in RelationInitIndexAccessInfo() so that it runs when the > relation cache first loads the index, put it into a new routine > RelationCheckValidity() and call that from ... I don't know, some > other place that runs whenever we access indexes but not when we > rebuild them. > > 3's probably a better idea, if you can find a reasonable place to call > it from. I'm thinking some time around the time the executor acquires > locks, but using a flag in the relcache entry to make sure it doesn't > run the check again if there were no warnings last time (so one > successful version check turns the extra work off for the rest of this > relcache entry's lifetime). I think it'd be a feature, not a bug, if > it gave you the warning every single time you executed a query using > an index that has a mismatch... but a practical alternative would be > to check only once per index and that probably makes more sense.
I tried to dig into approach #3. I think that trying to perform this check when the executor acquires locks won't work well with at least with partitioned table. I instead tried to handle that in get_relation_info(), adding a flag in the relcache to emit each warning only once per backend lifetime, and this seems to work quite well. I think that on top of that, we should make sure that non-full vacuum and analyze also emit such warnings, so that autovacuum can spam the logs too.
