On Thu, Mar 14, 2019 at 4:00 AM Heikki Linnakangas <[email protected]> wrote: > Oh yeah, that makes perfect sense. I wonder why we haven't done it like > that before? The new page split logic makes it more likely to help, but > even without that, I don't see any downside.
The only downside is that we spend a few extra cycles, and that might not work out. This optimization would have always worked, though. The new page split logic clearly makes checking the high key in the "continuescan" path an easy win. > I find it a bit confusing, that the logic is now split between > _bt_checkkeys() and _bt_readpage(). For a forward scan, _bt_readpage() > does the high-key check, but the corresponding "first-key" check in a > backward scan is done in _bt_checkkeys(). I'd suggest moving the logic > completely to _bt_readpage(), so that it's in one place. With that, > _bt_checkkeys() can always check the keys as it's told, without looking > at the LP_DEAD flag. Like the attached. I'm convinced. I'd like to go a bit further, and also pass tupnatts to _bt_checkkeys(). That makes it closer to the similar _bt_check_rowcompare() function that _bt_checkkeys() must sometimes call. It also allows us to only call BTreeTupleGetNAtts() for the high key, while passes down a generic, loop-invariant IndexRelationGetNumberOfAttributes() value for non-pivot tuples. I'll do it that way in the next revision. Thanks -- Peter Geoghegan
