> On Mar 23, 2026, at 08:44, Chao Li <[email protected]> wrote: > > > >> On Mar 23, 2026, at 08:42, Chao Li <[email protected]> wrote: >> >> >> >>> On Mar 4, 2026, at 08:49, Xuneng Zhou <[email protected]> wrote: >>> >>> Hi, >>> >>> On Thu, Feb 26, 2026 at 9:13 AM Chao Li <[email protected]> wrote: >>>> >>>> >>>> >>>>> On Feb 25, 2026, at 18:13, Kirill Reshke <[email protected]> wrote: >>>>> >>>>> 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. >>> >>> I think that the break is a small improvement. >>> >>> Once has_interval_ops becomes true, the loop has already learned >>> everything it needs for the optional hint, so scanning the remaining >>> key attributes does not change behavior. In that sense, your version >>> is slightly better than the pre-d70b176 code. >>> >>> AFAICS, the bug is that the corruption report was incorrectly gated on >>> finding INTERVAL_BTREE_FAM_OID; moving the ereport(ERROR) out of the >>> loop would fix it. >>> >>> LGTM from my side. >>> >> >> Cool. >> >> A gentle ping as this fixes a regression from d70b176. >> >> PFA v2 - just rebase, nothing changed. >> > > Sorry, forgot to attach the patch file. Here comes it. > > Best regards, > -- > Chao Li (Evan) > HighGo Software Co., Ltd. > https://www.highgo.com/ > > > > > <v2-0001-amcheck-fix-missing-corruption-error-in-allequali.patch>
Rebased to v3. Nothing changed. Hopefully gets noticed. This is really a regression bug. Kirill has confirmed the bug is an oversight of d70b176. BTW: the CF entry is https://commitfest.postgresql.org/patch/6534/. Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
v3-0001-amcheck-fix-missing-corruption-error-in-allequali.patch
Description: Binary data
