On Sun, Mar 29, 2020 at 10:16:53PM -0400, James Coleman wrote:
On Sun, Mar 29, 2020 at 9:44 PM Tomas Vondra
<tomas.von...@2ndquadrant.com> wrote:

Hi,

Attached is a slightly reorganized patch series. I've merged the fixes
into the appropriate matches, and I've also combined the two patches
adding incremental sort paths to additional places in planner.

A couple more comments:


1) I think the GUC documentation in src/sgml/config.sgml is a bit too
detailed, compared to the other enable_* GUCs. I wonder if there's a
better place where to move the details. What about adding some examples
and explanation to perform.sgml?

I'll take a look at that and include in a patch series tomorrow.

2) Looking at the explain output, the verbose mode looks like this:

test=# explain (verbose, analyze) select a from t order by a, b, c;
                                                                           
QUERY PLAN
--------------------------------------------------------------------------------------------------------------------------------------------------------------
  Gather Merge  (cost=66.31..816072.71 rows=8333226 width=24) (actual 
time=4.787..20092.555 rows=10000000 loops=1)
    Output: a, b, c
    Workers Planned: 2
    Workers Launched: 2
    ->  Incremental Sort  (cost=66.28..729200.36 rows=4166613 width=24) (actual 
time=1.308..14021.575 rows=3333333 loops=3)
          Output: a, b, c
          Sort Key: t.a, t.b, t.c
          Presorted Key: t.a, t.b
          Full-sort Groups: 4169 Sort Method: quicksort Memory: avg=30kB 
peak=30kB
          Presorted Groups: 4144 Sort Method: quicksort Memory: avg=128kB 
peak=138kB
          Worker 0:  actual time=0.766..16122.368 rows=3841573 loops=1
  Full-sort Groups: 6871 Sort Method: quicksort Memory: avg=30kB peak=30kB
            Presorted Groups: 6823 Sort Method: quicksort Memory: avg=132kB 
peak=141kB
          Worker 1:  actual time=1.986..16189.831 rows=3845490 loops=1
  Full-sort Groups: 6874 Sort Method: quicksort Memory: avg=30kB peak=30kB
            Presorted Groups: 6847 Sort Method: quicksort Memory: avg=130kB 
peak=139kB
          ->  Parallel Index Scan using t_a_b_idx on public.t  
(cost=0.43..382365.92 rows=4166613 width=24) (actual time=0.040..9808.449 
rows=3333333 loops=3)
                Output: a, b, c
                Worker 0:  actual time=0.048..11275.178 rows=3841573 loops=1
                Worker 1:  actual time=0.041..11314.133 rows=3845490 loops=1
  Planning Time: 0.166 ms
  Execution Time: 25135.029 ms
(22 rows)

There seems to be missing indentation for the first line of worker info.

Working on that too.

I'm still not quite convinced we should be printing two lines - I know
you mentioned the lines might be too long, but see how long the other
lines may get ...

All right, I give in :)

Do you think non-workers (both the leader and non-parallel plans)
should also move to one line?


I think we should use the same formatting for both cases, so yes.

FWIW I forgot to mention I tweaked the INSTRUMENT_SORT_GROUP macro a
bit, by moving the if condition in it. That makes the calls easier.

3) I see the new nodes (plan state, ...) have "presortedCols" which does
not indicate it's a "number of". I think we usually prefix names of such
fields "n" or "num". What about "nPresortedCols"? (Nitpicking, I know.)

I can fix this too.

Also I noticed a few compiler warnings I'll fixup in tomorrow's reply.


OK

regards

--
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


Reply via email to