| 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 |
| 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 |