Hi, On 2019-03-18 22:35:05 +1300, David Rowley wrote: > The commit message in 8586bf7ed mentions: > > > Subsequent commits will incrementally abstract table access > > functionality to be routed through table access methods. That change > > is too large to be reviewed & committed at once, so it'll be done > > incrementally. > > and looking at [1] I see patch 0004 introduces some changes in > nodeTidscan.c to call a new tableam API function named > heapam_fetch_row_version. I see this function does have a ItemPointer > argument, so I guess we must be keeping those as unique row > identifiers in the API.
Right, we are. At least for now - there's some discussions around allowing different format for TIDs, to allow things like index organized tables, but that's for later. > Patch 0001 does change the signature of heap_setscanlimits() (appears > to be committed already), and then in 0010 the only code that calls > heap_setscanlimits() (IndexBuildHeapRangeScan()) is moved and renamed > to heapam_index_build_range_scan() and set to be called via the > index_build_range_scan TableAmRoutine method. So it looks like out of > that patch series nothing is there to allow you to access > heap_setscanlimits() directly via the TableAmRoutine API, so perhaps > for this to work heap_setscanlimits will need to be interfaced, > however, I'm unsure if that'll violate any assumptions that Andres > wants to keep out of the API... I was kinda hoping to keep block numbers out of the "main" APIs, to avoid assuming everything is BLCKSZ based. I don't have a particular problem allowing an optional setscanlimits type callback that works with block numbers. The planner could check its presence and just not build tid range scans if not present. Alternatively a bespoke scan API for tid range scans, like the later patches in the tableam series for bitmap, sample, analyze scans, might be an option. Greetings, Andres Freund
