Re: bug: query returns different result with and without memoization.

From: "Yilin Zhang" <jiezhilove(at)126(dot)com>
To: "Richard Guo" <guofenglinux(at)gmail(dot)com>
Cc: "David Rowley" <dgrowleyml(at)gmail(dot)com>, "Jacob Brazeal" <jacob(dot)brazeal(at)gmail(dot)com>, "PostgreSQL Hackers" <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: bug: query returns different result with and without memoization.
Date: 2026-09-11 07:09:59
Message-ID: a91e0c0.4f6b.1a08f4d3e0a.Coremail.jiezhilove@126.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

At 2026-07-28 17:53:00, "Richard Guo" <guofenglinux(at)gmail(dot)com> wrote:
>along with any Param whose type has no hash opclass. Today each of
>those gets a working Memoize node that simply flushes the cache when
>the Param changes. So I'm concerned about the plan regression in
>cases that never had a problem.
>...
>So I'd like to propose the attached patch. Rather than looking for
>every place a Param might be used, it requires that every PARAM_EXEC
>Param appearing in a cache key expression is also a cache key in its
>own right. Then matching the keys implies matching the Param, and the
>set we have to examine is bounded, because it's just the key list we
>already built.
>
>The patch then also makes cache keys of the Params used outside the
>cache keys: in the relation's base quals, its targetlist, and on the
>inner side of the join clauses. It's what stops the cache being
>purged on every invocation of a correlated subplan. It's only an
>optimization, so it's ok if we miss a place a Param might be used.
>
>- Richard

Hi,
Thank you for working on this bug.

+ foreach(lc, params)
+ {
+ if (!memoize_add_cache_key((Node *) lfirst(lc), param_exprs, operators,
+ binary_mode))
{
- *operators = lappend_oid(*operators, typentry->eq_opr);
- *param_exprs = lappend(*param_exprs, expr);
+ list_free(*operators);
+ list_free(*param_exprs);
+ return false;
}
+ }

From the earlier discussion we learned that when a Param's type has no
hash opclass (geometric types such as point and box, domains over such
types, and custom types), memoize_add_cache_key() fails and the Memoize
path is abandoned entirely. Among types commonly used in production,
point has no hash or btree opclass at all, which can increase the scan
cost severalfold.

Building on your v1-0001-Fix-wrong-results-from-Memoize-with-a-Param-insid.patch,
I made some changes:

For unhashable buried Params, keep the attempt to add them as bare cache
keys; when that fails, record them in unhashable_params and exclude them
from keyparamids at createplan time, so that when such a Param changes it
falls into the executor's existing bms_nonempty_difference(chgParam,keyparamids)
difference set and triggers cache_purge_all().

I tested some types without hash opclass; the performance is basically
unchanged while the results remain correct.

Best regards,

--

Yilin Zhang

Attachment Content-Type Size
v2-0001-Fix-wrong-results-from-Memoize-with-a-Param-insid.patch application/octet-stream 30.4 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message zhang ziming 2026-09-11 07:14:28 回复: [PATCH v1] postgres_fdw: Fix local costing of remote quals after semi-joins
Previous Message Peter Smith 2026-09-11 07:04:10 Re: Review items for EXCEPT TABLE publication