Re: [PATCH] Incremental sort (was: PoC: Partial sort)

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?

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.

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 ...

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.)

My TODO for this patch is this:

- review the costing (I think the estimates are OK, but I recall I
haven't been entirely happy with how it's broken into functions.)

- review the tuplesort changes (the memory contexts etc.)

- do more testing of performance impact on planning

regards

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