On Wed, 25 Feb 2026 at 11:59, Chao Li <[email protected]> wrote: > > > > > On Feb 25, 2026, at 14:43, Kirill Reshke <[email protected]> wrote: > > > > On Wed, 25 Feb 2026 at 08:12, Chao Li <[email protected]> wrote: > >> > >> 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/ > >> > > > > > > uff, this looks like a clear oversight of d70b176. > > > > Before d70b176 it was like this: > > > > https://github.com/postgres/postgres/blame/fb9dff76635d4c32198f30a3cb503588d557d156/contrib/amcheck/verify_nbtree.c#L386-L399 > > > > > > Thanks for pointing out the origin code that seems to prove my fix is > correct. But my patch adds “break” in the “for” loop, which makes it slightly > better than the original version. >
OK LGTM then -- Best regards, Kirill Reshke
