Hi, Thanks!
On 2019-01-22 11:51:57 +1100, Haribabu Kommi wrote: > Attached the patch for removal of scan_update_snapshot > and also the rebased patch of reduction in use of t_tableOid. I'll soon look at the latter. > > - consider removing table_gimmegimmeslot() > > - add substantial docs for every callback > > > > Will work on the above two. I think it's easier if I do the first, because I can just do it while rebasing, reducing unnecessary conflicts. > > While I saw an initial attempt at writing smgl docs for the table AM > > API, I'm not convinced that's the best approach. I think it might make > > more sense to have high-level docs in sgml, but then do all the > > per-callback docs in tableam.h. > > > > OK, I will update the sgml docs accordingly. > Index AM has per callback docs in the sgml, refactor them also? I don't think it's a good idea to tackle the index docs at the same time - this patchset is already humongously large... > diff --git a/src/backend/access/heap/heapam_handler.c > b/src/backend/access/heap/heapam_handler.c > index 62c5f9fa9f..3dc1444739 100644 > --- a/src/backend/access/heap/heapam_handler.c > +++ b/src/backend/access/heap/heapam_handler.c > @@ -2308,7 +2308,6 @@ static const TableAmRoutine heapam_methods = { > .scan_begin = heap_beginscan, > .scan_end = heap_endscan, > .scan_rescan = heap_rescan, > - .scan_update_snapshot = heap_update_snapshot, > .scan_getnextslot = heap_getnextslot, > > .parallelscan_estimate = table_block_parallelscan_estimate, > diff --git a/src/backend/executor/nodeBitmapHeapscan.c > b/src/backend/executor/nodeBitmapHeapscan.c > index 59061c746b..b48ab5036c 100644 > --- a/src/backend/executor/nodeBitmapHeapscan.c > +++ b/src/backend/executor/nodeBitmapHeapscan.c > @@ -954,5 +954,9 @@ ExecBitmapHeapInitializeWorker(BitmapHeapScanState *node, > node->pstate = pstate; > > snapshot = RestoreSnapshot(pstate->phs_snapshot_data); > - table_scan_update_snapshot(node->ss.ss_currentScanDesc, snapshot); > + Assert(IsMVCCSnapshot(snapshot)); > + > + RegisterSnapshot(snapshot); > + node->ss.ss_currentScanDesc->rs_snapshot = snapshot; > + node->ss.ss_currentScanDesc->rs_temp_snap = true; > } I was rather thinking that we'd just move this logic into table_scan_update_snapshot(), without it invoking a callback. Greetings, Andres Freund