Re: Useless LEFT JOIN breaks MIN/MAX optimization

From: Alena Rybakina <a(dot)rybakina(at)postgrespro(dot)ru>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, m(dot)zhilin(at)postgrespro(dot)ru
Subject: Re: Useless LEFT JOIN breaks MIN/MAX optimization
Date: 2025-05-13 01:47:33
Message-ID: d8f4c695-141a-453a-a230-71fd0ab1ba1e@postgrespro.ru
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 12.05.2025 14:05, Tom Lane wrote:
> Robert Haas<robertmhaas(at)gmail(dot)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

Attachment Content-Type Size
min_max.diff.no-cfbot text/plain 5.5 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message J. Javier Maestro 2025-05-13 03:14:59 fix: propagate M4 env variable to flex subprocess
Previous Message Aya Iwata (Fujitsu) 2025-05-13 01:08:34 RE: [WIP]Vertical Clustered Index (columnar store extension) - take2