On Tue, Oct 10, 2023 at 09:35:45PM -0700, Peter Geoghegan wrote:
> On Tue, Oct 10, 2023 at 8:51 PM Peter Geoghegan <pg@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.