Subject: Re: Quadratic planning time for ordered paths over partitioned tables

From: Alexander Kuzmenkov <[email-masked]>
Date: Wed, 22 Jan 2025 17:50:37 +0100

On Wed, Jan 22, 2025 at 5:36 PM Alvaro Herrera <[email-masked]> wrote:
> I think this is closely related to the work Yuya Watari has been doing
> at
> https://postgr.es/m/[email-masked]
> Perhaps you could contribute by reviewing that patch series.

Yeah, I'm referencing this one in my email, but it's a series of
patches that does a major refactoring changing thousands of lines. I'm
not sure when or if it's going to land, do you think applying a quick
fix first would make sense? It makes trivial changes in one function.

---

From: Alvaro Herrera <[email-masked]>
Date: Wed, 22 Jan 2025 18:05:13 +0100

> fix first would make sense? It makes trivial changes in one function.

I think it might go faster if you try it out and review it.

Which problems did you notice that make you think it's not committable?
Maybe you can propose solutions for those problems.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/

---

From: Tom Lane <[email-masked]>
Date: Wed, 22 Jan 2025 12:15:23 -0500

> fix first would make sense? It makes trivial changes in one function.

That "quick fix" seems extremely horrid: since there's only one static
cache variable for all ECs, it's going to help only one very specific
call pattern, which could easily get broken by unrelated changes.
Also, maybe my performance instincts are rooted in old hardware, but
I don't trust integer division with a variable divisor to be cheap.
So ISTM this could as easily make things worse as better.  If you
offered something that was less obviously a kluge, maybe we could
use it.

Really I'd think the right place to be fixing this is at a higher
level.  Where are the repetitive find_ec_member_matching_expr calls
coming from?

			regards, tom lane

---

From: Alexander Kuzmenkov <[email-masked]>
Date: Wed, 22 Jan 2025 18:57:46 +0100

> level.  Where are the repetitive find_ec_member_matching_expr calls

I'm not aware of the repeated calls for the same child, and I mostly
see it called once for every partition from
prepare_sort_from_pathkeys(). For this case, fixing it at the high
level would require something like MergeAppend building a mapping of
Pathkey -> EM for all its children upfront, then passing this down to
the prepare_sort_from_pathkeys().

The other patch series focuses on this being called from the
generate_join_implied_equalities(). That call site could probably use
the same approach with building a mapping before generating the
partitionwise join paths.

---

From: Aleksander Alekseev <[email-masked]>
Date: Fri, 24 Jan 2025 16:15:19 +0300

Hi,

> fix first would make sense? It makes trivial changes in one function.

It looks like the author keeps the patch set up to date. Although he
proposes a complicated refactoring this is better than "quick and
dirty fix" as you put it, IMO. So I suggest focusing on this patch
set. On top of that somehow I doubt we will find a committer willing
to be responsible for merging anything quick and dirty anyway.

Did you consider checking if the referenced patchset addresses the
issue you described?

-- 
Best regards,
Aleksander Alekseev