On Thu, 11 Jul 2019 at 10:22, David Rowley <david.row...@2ndquadrant.com> wrote: > So it seems that the plan is to insist that TIDs are tuple identifiers > for all table AMs, for now. If that changes in the future, then so be > it, but I don't think that's cause for delaying any work on TID Range > Scans. Also from scanning around tableam.h, I see that there's no > shortage of usages of BlockNumber, so it seems reasonable to assume > table AMs must use blocks... It's hard to imagine moving away from > that given that we have shared buffers. > > We do appear to have some table AM methods that are optional, although > I'm not sure where the documentation is about that. For example, in > get_relation_info() I see: > > info->amhasgetbitmap = amroutine->amgetbitmap != NULL && > relation->rd_tableam->scan_bitmap_next_block != NULL; > > so it appears that at least scan_bitmap_next_block is optional. > > I think what I'd do would be to add a table_setscanlimits API method > for table AM and perhaps have the planner only add TID range scan > paths if the relation has a non-NULL function pointer for that API > function. It would be good to stick a comment at least in tableam.h > that mentions that the callback is optional. That might help a bit > when it comes to writing documentation on each API function and what > they do.
Hi. Here's a new patch. Summary of changes compared to last time: - I've added the additional "scan_setlimits" table AM method. To check whether it's implemented in the planner, I have added an additional "has_scan_setlimits" flag to RelOptInfo. It seems to work. - I've also changed nodeTidrangescan to not require anything from heapam now. - To simply the patch and avoid changing heapam, I've removed the backward scan support (which was needed for FETCH LAST/PRIOR) and made ExecSupportsBackwardScan return false for this plan type. - I've removed the vestigial passing of "direction" through nodeTidrangescan. If my understanding is correct, the direction passed to TidRangeNext will always be forward. But I did leave FETCH LAST/PRIOR in the regression tests (after adding SCROLL to the cursor). The patch now only targets the simple use case of restricting the range of a table to scan. I think it would be nice to eventually support the other major use cases of ORDER BY ctid ASC/DESC and MIN/MAX(ctid), but that can be another feature... Edmund
v8-0001-Add-a-new-plan-type-Tid-Range-Scan-to-support-range-.patch
Description: Binary data