Is select_outer_pathkeys_for_merge() too strict now we have Incremental Sorts?

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Is select_outer_pathkeys_for_merge() too strict now we have Incremental Sorts?
Date: 2022-07-20 03:02:36
Message-ID: CAApHDvrtZu0PHVfDPFM4Yx3jNR2Wuwosv+T2zqa7LrhhBr2rRg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hackers,

Currently, if we have a query such as:

SELECT a,b, COUNT(*)
FROM a
INNER JOIN b on a.a = b.x
GROUP BY a,b
ORDER BY a DESC, b;

With enable_hashagg = off, we get the following plan:

QUERY PLAN
---------------------------------------
GroupAggregate
Group Key: a.a, a.b
-> Sort
Sort Key: a.a DESC, a.b
-> Merge Join
Merge Cond: (a.a = b.x)
-> Sort
Sort Key: a.a
-> Seq Scan on a
-> Sort
Sort Key: b.x
-> Seq Scan on b

We can see that the merge join picked to sort the input on a.a rather
than a.a DESC. This is due to the way
select_outer_pathkeys_for_merge() only picks the query_pathkeys as a
prefix of the join pathkeys if we can find all of the join pathkeys in
the query_pathkeys.

I think we can relax this now that we have incremental sort. I think
a better way to limit this is to allow a prefix of the query_pathkeys
providing that covers *all* of the join pathkeys. That way, for the
above query, it leaves it open for the planner to do the Merge Join by
sorting by a.a DESC then just do an Incremental Sort to get the
GroupAggregate input sorted by a.b.

I've attached a patch for this and it changes the plan for the above query to:

QUERY PLAN
----------------------------------------
GroupAggregate
Group Key: a.a, a.b
-> Incremental Sort
Sort Key: a.a DESC, a.b
Presorted Key: a.a
-> Merge Join
Merge Cond: (a.a = b.x)
-> Sort
Sort Key: a.a DESC
-> Seq Scan on a
-> Sort
Sort Key: b.x DESC
-> Seq Scan on b

The current behaviour is causing me a bit of trouble in plan
regression for the ORDER BY / DISTINCT aggregate improvement patch
that I have pending.

Is there any reason that we shouldn't do this?

David

Attachment Content-Type Size
make_select_outer_pathkeys_for_merge_less_strict.patch text/plain 4.9 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Kapila 2022-07-20 03:11:21 Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Previous Message Masahiko Sawada 2022-07-20 01:58:16 Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns