On Tue, Oct 10, 2023 at 09:35:45PM -0700, Peter Geoghegan wrote: > On Tue, Oct 10, 2023 at 8:51 PM Peter Geoghegan <p...@bowt.ie> wrote: > > I don't see any reason to delay committing your fix. The issue that > > you've highlighted is exactly the kind of issue that I anticipated > > might happen at some point. This seems straightforward. > > BTW, we don't need to recommend the heapallindexed option in the > release notes. Calling bt_check_index() will reliably indicate that > corruption is present when called against existing interval_ops > indexes once your fix is in. Simply having an index whose metapage's > allequalimage field is spuriously set to true will be recognized as > corruption right away. Obviously, this will be no less true with an > existing interval_ops index that happens to be completely empty.
Interesting. So, >99% of interval-type indexes, even ones WITH (deduplicate_items=off), will get amcheck failures. The <1% of exceptions might include indexes having allequalimage=off due to an additional column, e.g. a two-column (interval, numeric) index. If interval indexes are common enough and "pg_amcheck --heapallindexed" failures from $SUBJECT are relatively rare, that could argue for giving amcheck a special case. Specifically, downgrade its "metapage incorrectly indicates that deduplication is safe" from ERROR to WARNING for interval_ops only. Without that special case (i.e. with the v1 patch), the release notes should probably resemble, "After updating, run REINDEX on all indexes having an interval-type column." There's little point in recommending pg_amcheck if >99% will fail. I'm inclined to bet that interval-type indexes are rare, so I lean against adding the amcheck special case. It's not a strong preference. Other opinions? If users want to reduce their exposure now, they could do "ALTER INDEX ... SET (deduplicate_items = off)" and then REINDEX any indexes already failing "pg_amcheck --heapallindexed". allequalimage will remain wrong but have no ill effects beyond making amcheck fail. Another REINDEX after the update would let amcheck pass.
