Re: Hybrid Hash/Nested Loop joins and caching results from subplans

On Thu, 3 Sep 2020 at 01:49, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com> wrote:
>
> On 2020-Sep-02, David Rowley wrote:
>
> > v7 (Separate Result Cache node)
> > Query 1:
> > Execution Time: 894.003 ms
> >
> > Query 2:
> > Execution Time: 854.950 ms
>
> > v7 + hacks_V3 (caching done in Nested Loop)
> > Query 1:
> > Execution Time: 770.470 ms
> >
> > Query 2
> > Execution Time: 779.181 ms
>
> Wow, this is a *significant* change.

Yeah, it's more than I thought it was going to be. It seems I
misthought in [1] where I mentioned:

> With a hit ratio of 90% we'll
> only pull 10% of tuples through the additional node, so that's about
> 1.2 nanoseconds per tuple, or 1.2 milliseconds per million tuples. It
> might become hard to measure above the noise. More costly inner scans
> will have the planner choose to Result Cache with lower estimated hit
> ratios, but in that case, pulling the tuple through the additional
> node during a cache miss will be less noticeable due to the more
> costly inner side of the join.

This wasn't technically wrong. I just failed to consider that a cache
hit when the cache is built into Nested Loop requires looking at no
other node. The tuples are right there in the cache, 90% of the time,
in this example. No need to execute any nodes to get at them.

I have come around a bit to Andres' idea. But we'd need to display the
nested loop node as something like "Cacheable Nested Loop" in EXPLAIN
so that we could easily identify what's going on. Not sure if the word
"Hash" would be better to inject in the name somewhere rather than
"Cacheable".

I've not done any further work to shift the patch any further in that
direction yet. I know it's going to be quite a bit of work and it
sounds like there are still objections in both directions. I'd rather
everyone agreed on something before I go to the trouble of trying to
make something committable with Andres' way.

Tom, I'm wondering if you'd still be against this if Nested Loop
showed a different name in EXPLAIN when it was using caching? Or are
you also concerned about adding unrelated code into nodeNestloop.c?
If so, I'm wondering if adding a completely new node like
nodeNestcacheloop.c. But that's going to add lots of boilerplate code
that we'd get away with not having otherwise.

In the meantime, I did change a couple of things with the current
separate node version. It's just around how the path stuff works in
the planner. I'd previously modified try_nestloop_path() to try a
Result Cache, but I noticed more recently that's not how it's done for
Materialize. So in the attached, I've just aligned it to how
non-parameterized Nested Loops with a Materialized inner side work.

David

[1] https://www.postgresql.org/message-id/CAApHDvrX9o35_WUoL5c5arJ0XbJFN-cDHckjL57-PR-Keeypdw@mail.gmail.com