On Wed, 13 Jan 2021 at 15:38, Edmund Horner <ejrh00@gmail.com> wrote:
> Thanks for updating the patch. I'd be very happy if this got picked up again, and I'd certainly follow along and do
somereview.
Likewise here. I this patch was pretty close so it seems a shame to
let it slip through the cracks.
I spoke to Andres off-list about this patch. He mentioned that he
wasn't too keen on seeing the setscanlimits being baked into the table
AM API. He mentioned that he'd rather not assume too much about table
AMs having all of their tids in a given range consecutively on a set
of pages. That seems reasonable to me. He suggested that we add a
new callback that just allows a range of ItemPointers to scan and
leave it up to the implementation to decide which pages should be
scanned to fetch the tuples within the given range. I didn't argue. I
just went and coded it all, hopefully to Andres' description. The new
table AM callback is optional.
I've implemented this in the attached.
I also took the time to support backwards TID Range scans and added a
few more tests to test rescans. I just found it annoying that TID
Scans supported backwards scans and TID Range scans did not.
The 0002 patch is the guts of it. The 0001 patch is an existing bug
that needs to be fixed before 0002 could go in (backwards TID Range
Scans are broken without this). I've posted separately about this bug
in [1]
I also didn't really like the idea of adding possibly lots of bool
fields to RelOptInfo to describe what the planner can do in regards to
what the given table AM supports. I know that IndexOptInfo has such
a set of bool fields. I'd rather not repeat that, so I just went with
a single int field named "amflags" and just added a single constant to
define a flag that specifies if the rel supports scanning ranges of
TIDs.
Edmund, will you get time to a look at this?
David
[1] https://www.postgresql.org/message-id/CAApHDvpGc9h0_oVD2CtgBcxCS1N-qDYZSeBRnUh+0CWJA9cMaA@mail.gmail.com