On Thu, 2 Jan 2025 at 17:38, Peter Geoghegan <pg@bowt.ie> wrote:
>
> Attached patch goes a bit further with simplifying _bt_first's
> handling of seizing the parallel scan. This continues recent work from
> commits 4e6e375b and b5ee4e52.
>
> Aside from requiring less code, the new structure relieves _bt_first
> from having separate calls to _bt_start_array_keys for the serial and
> parallel cases. It also places emphasis on the idea that it's expected
> that calls to _bt_first will go through _bt_readfirstpage (not
> _bt_readnextpage). While it is of course possible for a parallel
> scan's _bt_first to call _bt_readnextpage instead, that is the
> exception, not the rule.
Apart from comments on comment contents and placement, no specific issues:
> + *
> + * Initialize arrays during first (unscheduled) primitive index scan.
> + */
I think this could be more clear about why these conditions indicate
the first unscheduled primitive index scan.
> /*
> * Count an indexscan for stats, now that we know that we'll call
> * _bt_search/_bt_endpoint below
> + *
> + * _bt_readfirstpage will release the seized parallel scan, if any.
> */
I don't understand the placement of that comment, as it's quite far
away from any parallel scan related code and it's very unrelated to
the index scan statistics.
If this needs to be added, I think I'd put it next to the call to
_bt_parallel_seize().
Kind regards,
Matthias van de Meent
Neon (https://neon.tech)
