Re: A performance issue with Memoize

Richard Guo <guofenglinux@gmail.com>

From: Richard Guo <guofenglinux@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2024-01-26T06:02:54Z
Lists: pgsql-hackers
On Fri, Jan 26, 2024 at 12:18 PM David Rowley <dgrowleyml@gmail.com> wrote:

> On Fri, 26 Jan 2024 at 16:51, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > >> However ... it seems like we're not out of the woods yet.  Why
> > >> is Richard's proposed test case still showing
> > >> +         ->  Memoize (actual rows=5000 loops=N)
> > >> +               Cache Key: t1.two, t1.two
> > >> Seems like there is missing de-duplication logic, or something.
> >
> > > This seems separate and isn't quite causing the same problems as what
> > > Richard wants to fix so I didn't touch this for now.
> >
> > Fair enough, but I think it might be worth pursuing later.
>
> Here's a patch for that.


At first I wondered if we should assume that the same param expr must
have the same equality operator. If not, we should also check the
operator to tell if the cache key is a duplicate, like

-           if (!list_member(*param_exprs, expr))
+           if (!list_member(*param_exprs, expr) ||
+               !list_member_oid(*operators, hasheqoperator))

But after looking at how rinfo->left_hasheqoperator/right_hasheqoperator
is set, it seems we can assume that: the operator is from the type cache
entry which is fetched according to the expr datatype.

So I think the patch makes sense.  +1.

Thanks
Richard

Commits

  1. Attempt to fix newly added Memoize regression test

  2. Compare varnullingrels too in assign_param_for_var().

  3. De-dupicate Memoize cache keys

  4. Improve NestLoopParam generation for lateral subqueries

  5. Avoid sharing PARAM_EXEC slots between different levels of NestLoop.