Hi, On 2019-03-15 18:42:40 +1300, Edmund Horner wrote: > I've had to adapt it to use the table scan API. I've got it compiling > and passing tests, but I'm uneasy about some things that still use the > heapam API. > > 1. I call heap_setscanlimits as I'm not sure there is a tableam > equivalent.
There used to be, but it wasn't clear that it was useful. In core pg the only caller are index range scans, and those are - in a later patch in the series - moved into the AM as well, as they need to deal with things like HOT. > 2. I'm not sure whether non-heap tableam implementations can also be > supported by my TID Range Scan: we need to be able to set the scan > limits. There may not be any other implementations yet, but when > there are, how do we stop the planner using a TID Range Scan for > non-heap relations? I've not yet looked through your code, but if required we'd probably need to add a new tableam callback. It'd be marked optional, and the planner could just check for its presence. A later part of the pluggable storage series does that for bitmap scans, perhaps it's worth looking at that? > 3. When fetching tuples, I see that nodeSeqscan.c uses > table_scan_getnextslot, which saves dealing with HeapTuples. But > nodeTidrangescan wants to do some checking of the block and offset > before returning the slot. So I have it using heap_getnext and > ExecStoreBufferHeapTuple. Apart from being heapam-specific, it's just > not as clean as the new API calls. Yea, that's not ok. Note that, since yesterday, nodeTidscan doesn't call heap_fetch() anymore (there's still a heap dependency, but that's just for heap_get_latest_tid(), which I'll move into execMain or such). > Ideally, we can get to to support general tableam implementations > rather than using heapam-specific calls. Any advice on how to do > this? Not yet - could you perhaps look at the bitmap scan patch in the tableam queue, and see if that gives you inspiration? - Andres