Hi,
While poking around the code in contrib/amcheck/verify_nbtree.c, I noticed the
following block:
```
if (allequalimage && !_bt_allequalimage(indrel, false))
{
bool has_interval_ops = false;
for (int i = 0; i <
IndexRelationGetNumberOfKeyAttributes(indrel); i++)
if (indrel->rd_opfamily[i] == INTERVAL_BTREE_FAM_OID)
{
has_interval_ops = true;
ereport(ERROR,
(errcode(ERRCODE_INDEX_CORRUPTED),
errmsg("index \"%s\" metapage
incorrectly indicates that deduplication is safe",
RelationGetRelationName(indrel)),
has_interval_ops
? errhint("This is known of
\"interval\" indexes last built on a version predating 2023-11.")
: 0));
}
}
```
My initial impression was that has_interval_ops was unneeded and could be
removed, as it is always true at the point of use. I originally thought this
would just be a tiny refactoring.
However, on second thought, I realized that having the ereport inside the for
loop is actually a bug. If allequalimage is set in the metapage but
_bt_allequalimage says it’s unsafe, we should report corruption regardless of
the column types. In the current code, if the index does not contain an
interval opfamily, the loop finishes without reaching the ereport, thus
silencing the corruption.
This patch moves the ereport out of the for loop. This ensures that corruption
is reported unconditionally, while keeping the interval-specific hint optional.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
v1-0001-amcheck-fix-missing-corruption-error-in-allequali.patch
Description: Binary data
