Re: [PATCH] Consider window functions when reordering GROUP BY items

Hi Henson,

Thanks for the review and for the test matrix, it was very helpful.
v4 is attached.

> 1. The missing operator error still happens with eager aggregation.

Fixed. The check in v3 was only in group_keys_reorder_by_pathkeys(),
but the partial aggregation goes through adjust_group_pathkeys_to_input(),
which took the input's DESC pathkey without checking the operator. Now
it stops there if the opfamily don't have the operator for that
direction. Your query returns 3 rows like master, and I added it to
eager_aggregate.sql.

> Would it work to offer the window's ordering as one more candidate,
> next to the orderings that get_useful_group_keys_orderings() already
> returns, and let the cost decide?

Yes, that works. preprocess_groupclause() is back to master, and the
window ordering is now just one more candidate in
get_useful_group_keys_orderings(). I also had to add it in
gather_grouping_paths(), otherwise the sort for the window ordering
is done by the leader above Gather Merge and the ea/eb query lose
most of the gain.

Numbers (5 runs, ms, -O2 with assertions):

master v3 v4
your tb/ts query 85-91 181-189 65-76
ea/eb window 223-257 106-123 117-138

Your tb/ts query is now in window.sql. While running check-world I
noticed the plan there depend on the visibility map, so the test
disables nestloop and hashjoin to keep it stable.

> 3. Planning time.

I re-measured the same way as you did. v4 is about the same as v3
(0.89-1.01x), the window query is ~13us more than master.

> 4. The slower cases come from the existing cost model.

Agree, I'd leave it for a separate patch. I reran cf7150-matrix.sql
on v4, it's 35 faster and 7 slower out of 42 changed plans, and the
slower ones all have the Index Scan + Incremental Sort shape you
described.

Two behavior changes from v3 that I should mention:

- The skipping of redundant sort clauses (e.g. "ORDER BY x USING ~<~,
x, y") is gone, since it was part of preprocess_groupclause()
changes. That query plans same as master now.
- The window ordering now follows enable_group_by_reordering, like the
other reorderings. In v3 it was applied always.

Regards,
Zhenglong Li