Re: Table AM Interface Enhancements
Melanie Plageman <melanieplageman@gmail.com>
On Thu, Apr 11, 2024 at 12:19 PM Melanie Plageman <melanieplageman@gmail.com> wrote: > > On Wed, Apr 10, 2024 at 5:21 PM Andres Freund <andres@anarazel.de> wrote: > > > > Hi, > > > > On 2024-04-10 16:50:44 -0400, Melanie Plageman wrote: > > > This brings up a question about the prefetching. We never had to have > > > this discussion for sequential scan streaming read because it didn't > > > (and still doesn't) do prefetching. But, if we push the streaming read > > > code down into the heap AM layer, it will be doing the prefetching. > > > So, do we remove the prefetching from acquire_sample_rows() and expect > > > other table AMs to implement it themselves or use the streaming read > > > API? > > > > The prefetching added to acquire_sample_rows was quite narrowly tailored to > > something heap-like - it pretty much required that block numbers to be 1:1 > > with the actual physical on-disk location for the specific AM. So I think > > it's pretty much required for this to be pushed down. > > > > Using a read stream is a few lines for something like this, so I'm not worried > > about it. I guess we could have a default implementation for block based AMs, > > similar what we have around table_block_parallelscan_*, but not sure it's > > worth doing that, the complexity is much lower than in the > > table_block_parallelscan_ case. > > This makes sense. > > I am working on pushing streaming ANALYZE into heap AM code, and I ran > into a few roadblocks. > > If we want ANALYZE to make the ReadStream object in heap_beginscan() > (like the read stream implementation of heap sequential and TID range > scans do), I don't see any way around changing the scan_begin table AM > callback to take a BufferAccessStrategy at the least (and perhaps also > the BlockSamplerData). I will also say that, had this been 6 months ago, I would probably suggest we restructure ANALYZE's table AM interface to accommodate read stream setup and to address a few other things I find odd about the current code. For example, I think creating a scan descriptor for the analyze scan in acquire_sample_rows() is quite odd. It seems like it would be better done in the relation_analyze callback. The relation_analyze callback saves some state like the callbacks for acquire_sample_rows() and the Buffer Access Strategy. But at least in the heap implementation, it just saves them in static variables in analyze.c. It seems like it would be better to save them in a useful data structure that could be accessed later. We have access to pretty much everything we need at that point (in the relation_analyze callback). I also think heap's implementation of table_beginscan_analyze() doesn't need most of heap_beginscan()/initscan(), so doing this instead of something ANALYZE specific seems more confusing than helpful. - Melanie
Commits
-
Remove extra comment at TableAmRoutine.scan_analyze_next_block
- 70a845c04a47 17.0 landed
-
revert: Generalize relation analyze in table AM interface
- 6377e12a5a52 17.0 landed
-
Revert: Allow table AM to store complex data structures in rd_amcache
- 922c4c461d21 17.0 landed
-
Revert: Allow table AM tuple_insert() method to return the different slot
- 8dd0bb84da7d 17.0 landed
-
Revert: Allow locking updated tuples in tuple_update() and tuple_delete()
- 193e6d18e553 17.0 landed
-
Revert: Let table AM insertion methods control index insertion
- da841aa4dc27 17.0 landed
-
Revert: Custom reloptions for table AM
- bc1e2092ebb8 17.0 landed
-
Provide a way block-level table AMs could re-use acquire_sample_rows()
- dd1f6b0c172a 17.0 landed
-
Custom reloptions for table AM
- 9bd99f4c26fe 17.0 landed
- c95c25f9af4b 17.0 landed
-
Use streaming I/O in ANALYZE.
- 041b96802efa 17.0 cited
-
Revert "Custom reloptions for table AM"
- 867cc7b6ddb9 17.0 landed
-
Let table AM insertion methods control index insertion
- b1484a3f1910 17.0 landed
-
Generalize relation analyze in table AM interface
- 27bc1772fc81 17.0 landed
-
Improve error message for tts_(virtual|minimal)_is_current_xact_tuple
- 41d3780d3d29 17.0 landed
-
Add comments on some MinimalTupleSlots methods usage
- 10baee0c95d1 17.0 landed
-
Add TupleTableSlotOps.is_current_xact_tuple() method
- 0997e0af273d 17.0 landed
-
Allow table AM tuple_insert() method to return the different slot
- c35a3fb5e067 17.0 landed
-
Allow table AM to store complex data structures in rd_amcache
- 02eb07ea89d2 17.0 landed