| From: | David Rowley <dgrowleyml(at)gmail(dot)com> |
|---|---|
| To: | Richard Guo <guofenglinux(at)gmail(dot)com> |
| Cc: | 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-07-27 12:46:18 |
| Message-ID: | CAApHDvoinY+=V8Ts=PMVS4xiwYDpofZpkD=hNK5iR+Xx+0HR9A@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Mon, 27 Jul 2026 at 20:46, Richard Guo <guofenglinux(at)gmail(dot)com> wrote:
> In the reported case, there is a cache key of "t2.hundred + t0.ten"
> and a separate filter "t1.twenty = t0.ten". The problem is that
> different (t2.hundred, t0.ten) pairs can produce the same cache-key
> value while the filter selects different rows.
>
> I think we can fix it by dropping from keyparamids any parameter that
> also appears outside the cache-key expressions. See the attached
> patch.
I'm not that keen on that being the fix for this. I think the correct
way to fix is to add the extra cache key. That way, we have at least
got a chance at costing Memoize correctly. I feel your method puts us
at risk of Memoize nodes suddenly having a terrible cache hit ratio in
the back branches. The alternative method of adding the extra key
does risk that Memoize is no longer chosen and some other (perhaps)
more suitable join method is selected instead. None of which is
particularly great in the back-branches, but I'd be quite nervous
about your method of flushing the cache each time a parameter like
this changes, as in the worst case scenario, Memoize might never get a
cache hit, and that won't be accounted for in the costs.
For the reported test case, with your patch I get:
-> Memoize (cost=0.30..0.36 rows=1 width=4) (actual
rows=0.05 loops=2000000)
Cache Key: (t2.hundred + t0.ten)
Cache Mode: logical
Estimates: capacity=100 distinct keys=100
lookups=10000 hit percent=99.00%
Hits: 1980000 Misses: 20000 Evictions: 19900
Overflows: 0 Memory Usage: 7kB
Whereas with my method:
-> Memoize (cost=0.30..0.36 rows=1 width=4) (actual
rows=0.05 loops=2000000)
Cache Key: (t2.hundred + t0.ten), t0.ten
Cache Mode: logical
Estimates: capacity=100 distinct keys=100
lookups=10000 hit percent=99.00%
Hits: 1999000 Misses: 1000 Evictions: 0
Overflows: 0 Memory Usage: 73kB
For reference, I was experimenting with the attached. What do you think?
David
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Fix-missing-Memoize-Cache-keys-for-base-qual-Para.patch | application/octet-stream | 7.3 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jonathan Gonzalez V. | 2026-07-27 13:06:34 | Re: [PATCH] Cover get_json_table_plan() with tests |
| Previous Message | Vladlen Popolitov | 2026-07-27 12:40:09 | Re: Bug and memory leaks with access to file links with long names (Windows, MSVS) |