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
-
Attempt to fix newly added Memoize regression test
- a3a836fb5e51 17.0 landed
-
Compare varnullingrels too in assign_param_for_var().
- 807369d80384 16.2 landed
- 5e444a2526cc 17.0 landed
-
De-dupicate Memoize cache keys
- bc397e5cdb31 17.0 landed
-
Improve NestLoopParam generation for lateral subqueries
- 2cca95e17546 17.0 landed
-
Avoid sharing PARAM_EXEC slots between different levels of NestLoop.
- 1db5667bac63 12.0 cited