Hi David,
Thanks for the review!
> 1. I'm confused by the following in explain.sql. We don't support
> parallel function scans, so the comments and setting of
> max_parallel_workers_per_gather seem bogus:
Function Scan isn't parallel-aware, but generate_series() is parallel
safe, so with debug_parallel_query = regress the whole plan runs in a
parallel worker. EXPLAIN only reads the storage information from the
leader's tuplestore, so the Storage line doesn't appear in that case.
Indeed, when I removed the setting and ran make check with
debug_parallel_query = regress, the Storage line disappeared from all
three of the new tests and the explain test failed.
So I kept the setting and rewrote the comment to make the reason
clearer.
> 2. In the following, 10000 tuples seems a little excessive to spill 64
> ...
> You could also move the existing "reset work_mem;" down after your
> tests. Your 10-row test won't spill with work_mem set to 64.
As you suggested, I changed it to 2500 rows, moved the existing
"reset work_mem;" down after the new tests, and removed the
"set work_mem to 64;" that I had added.
In v3, only the tests in 0002 have changed; 0001 is unchanged.
Regards,
Tatsuya Kawata
