Re: Useless LEFT JOIN breaks MIN/MAX optimization
Alena Rybakina <a.rybakina@postgrespro.ru>
From: Alena Rybakina <a.rybakina@postgrespro.ru>
To: Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>,
m.zhilin@postgrespro.ru
Date: 2025-05-13T01:47:33Z
Lists: pgsql-hackers
Attachments
- min_max.diff.no-cfbot (text/plain)
On 12.05.2025 14:05, Tom Lane wrote: > Robert Haas<robertmhaas@gmail.com> writes: >> But that's also assuming that you're correct here about how to descend >> through a JoinExpr, which I'm not quite sure whether is true. It's >> also assuming that we should solve the problem here rather than in >> some other part of the code e.g. the join removal code, and I'm not >> sure about that either. > The actual problem here is that remove_useless_joins hasn't run yet. > It's called inside query_planner which happens only after we do > preprocess_minmax_aggregates. > > So I think this patch is a dead end. It's not possible for it to > correctly predict whether remove_useless_joins will remove the join, > short of repeating all that work which we surely don't want. > (I'm a bit surprised that it hasn't visibly broken existing test cases.) To be honest, I was not completely sure about my decision at first and had no idea how to do it differently, so I submitted a request for "Advanced session feedback" to consider this patch. > It might be possible to move preprocess_minmax_aggregates to happen > after join removal, but I fear it'd require some pretty fundamental > rethinking of how it generates indexscan paths --- recursively > calling query_planner seems dubious. (But maybe that'd work? > The modified query should no longer contain aggs, so we wouldn't > recurse again.) I considered another approach using late optimization and ran into a problem where the planner could not find a partitioned table. It was a long time ago to be frankly, but the problem there was that the planner stored this information at a higher level. I can try to finish this. I attached a diff just in case. > preprocess_minmax_aggregates is pretty much of a hack anyway. > If you read the comments, it's just full of weird stuff that it > has to duplicate from other places, or things that magically work > because the relevant stuff isn't possible in this query, etc. > Maybe it's time to think about nuking it from orbit and doing a > fresh implementation in some other place that's a better fit. > I have no immediate ideas about what that should look like, other > than it'd be better if it happened after join removal. I didn't consider this and I'll think about it. Thanks for the feedback! Regards, Alena Rybakina Postgres Professional