Re: BitmapHeapScan streaming read user and prelim refactoring

Melanie Plageman <melanieplageman@gmail.com>

From: Melanie Plageman <melanieplageman@gmail.com>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Tomas Vondra <tomas.vondra@enterprisedb.com>, Thomas Munro <thomas.munro@gmail.com>, Heikki Linnakangas <hlinnaka@iki.fi>, Andres Freund <andres@anarazel.de>, Pg Hackers <pgsql-hackers@postgresql.org>, Nazir Bilal Yavuz <byavuz81@gmail.com>
Date: 2024-09-27T18:58:31Z
Lists: pgsql-hackers
On Mon, Aug 26, 2024 at 10:49 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Robert Haas <robertmhaas@gmail.com> writes:
> > On Wed, Jun 19, 2024 at 2:21 PM Melanie Plageman
> > <melanieplageman@gmail.com> wrote:
> >> If we want to make it possible to use no tools and only manually grep
> >> for struct members, that means we can never reuse struct member names.
> >> Across a project of our size, that seems like a very serious
> >> restriction. Adding prefixes in struct members makes it harder to read
> >> code -- both because it makes the names longer and because people are
> >> more prone to abbreviate the meaningful parts of the struct member
> >> name to make the whole name shorter.
>
> > I don't think we should go so far as to never reuse a structure member
> > name. But I also do use 'git grep' a lot to find stuff, and I don't
> > appreciate it when somebody names a key piece of machinery 'x' or 'n'
> > or something, especially when references to that thing could
> > reasonably occur almost anywhere in the source code. So if somebody is
> > creating a struct whose names are fairly generic and reasonably short,
> > I like the idea of using a prefix for those names. If the structure
> > members are things like that_thing_i_stored_behind_the_fridge (which
> > is long) or cytokine (which is non-generic) then they're greppable
> > anyway and it doesn't really matter. But surely changing something
> > like rs_flags to just flags is just making everyone's life harder:
>
> I'm with Robert here: I care quite a lot about the greppability of
> field names.  I'm not arguing for prefixes everywhere, but I don't
> think we should strip out prefixes we've already created, especially
> if the result will be to have extremely generic field names.

Okay, got it -- folks like the prefixes.
I'm picking this patch set back up again after a long pause and I will
restore all prefixes.

What does the rs_* in the HeapScanDescData stand for, though?

- Melanie



Commits

  1. Fix bitmapheapscan incorrect recheck of NULL tuples

  2. Increase default maintenance_io_concurrency to 16

  3. Separate TBM[Shared|Private]Iterator and TBMIterateResult

  4. Improve read_stream.c advice for dense streams.

  5. Increase default effective_io_concurrency to 16

  6. Delay extraction of TIDBitmap per page offsets

  7. Add lossy indicator to TBMIterateResult

  8. Move BitmapTableScan per-scan setup into a helper

  9. Add and use BitmapHeapScanDescData struct

  10. Fix bitmap table scan crash on iterator release

  11. Bitmap Table Scans use unified TBMIterator

  12. Add common interface for TBMIterators

  13. Make table_scan_bitmap_next_block() async-friendly

  14. Move EXPLAIN counter increment to heapam_scan_bitmap_next_block

  15. Refactor tidstore.c iterator buffering.

  16. BitmapHeapScan: Remove incorrect assert and reset field

  17. Change BitmapAdjustPrefetchIterator to accept BlockNumber

  18. BitmapHeapScan: Use correct recheck flag for skip_fetch

  19. BitmapHeapScan: Push skip_fetch optimization into table AM

  20. BitmapHeapScan: postpone setting can_skip_fetch

  21. BitmapHeapScan: begin scan after bitmap creation

  22. Fix EXPLAIN Bitmap heap scan to count pages with no visible tuples

  23. Remove redundant snapshot copying from parallel leader to workers

  24. Remove some obsolete smgrcloseall() calls.

  25. Remove the "snapshot too old" feature.

  26. Compute XID horizon for page level index vacuum on primary.