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 |
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 |