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.


> commit d70b17636ddf1ea2c71d1c7bc477372b36ccb66b
> Author: Tomas Vondra <[email protected]>
> Date:   Sat Mar 29 15:14:47 2025 +0100

>    amcheck: Move common routines into a separate module
...
>    Reviewed-By: Kirill Reshke <[email protected]>
>    Discussion: 
> https://postgr.es/m/45AC9B0A-2B45-40EE-B08F-BDCF5739D1E1%40yandex-team.ru

Oops.

Before d70b176 it was like this:

https://github.com/postgres/postgres/blame/fb9dff76635d4c32198f30a3cb503588d557d156/contrib/amcheck/verify_nbtree.c#L386-L399


-- 
Best regards,
Kirill Reshke


Reply via email to