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
isset 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
keepingthe interval-specific hint optional.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
