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

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