On Sun, Dec 1, 2024 at 10:15 AM yuansong <yyuans...@126.com> wrote: > I confirmed that there's an issue with prematurely ending the loop. When I > enter the if (unlikely(result == 0 && key->scantid != NULL)) block, low = 2, > mid = 4, and high = 4. At this point, the offset of insertstate->postingoff > is obtained from the mid value, which is 4.
I don't understand why you believe that there is a bug. What bug? Your patch seems to just add a micro-optimization. Your patch makes the robustness situation worse, not better: it will make it impossible to hit the "if (insertstate->postingoff != 0)" ereport() when (for whatever reason) there is index corruption that somehow leads to a leaf page with more than one "result == 0 && key->scantid != NULL" tuple. In other words, your patch actually *removes* the thing that I added to ameliorate/avoid problems such as your problem with a btree_xlog_insert record containing a spurious offset number during REDO. Of course there shouldn't be more than one single "if (insertstate->postingoff != 0)" index tuple on a leaf page that we're inserting onto, but, assuming that that invariant somehow becomes violated (due to corruption), why should we then just accept the first such overlapping index tuple within _bt_binsrch_insert? We should be (and are) more careful than that: we make sure that there isn't another "if (insertstate->postingoff != 0)" tuple, in passing -- even though doing so is theoretically unnecessary (in theory the database never has corruption, in practice it might, for many different reasons). -- Peter Geoghegan
