Re: A performance issue with Memoize

On Fri, Jan 26, 2024 at 12:18 PM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> On Fri, 26 Jan 2024 at 16:51, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)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