On 11/12/26 03:46, David Rowley wrote:
On Thu, 12 Nov 2026 at 01:15, Ilia Evdokimov
<[email protected]> wrote:
I found a case where Memoize rescans its inner plan even though the complete
result for the requested key is already cached.
When the cache exceeds its memory limit, Memoize removes older entries.
Removing an entry from the hash table can move other entries to different
buckets, including the entry currently being filled.
The code that stores a tuple accounts for this movement and updates
node->entry. However, its caller then marks the entry as complete using the old
local pointer:
Interesting. Human or AI found?
AI, of course.
I don't think any of our tests would have found this. Even if a test had
exposed the symptom, I suspect it would have been attributed to
something else rather than to this particular line.
AI drew my attention to the assignment, which made me look at it more
closely. At first glance, it seemed suspicious: cache_store_tuple() can
move the entry, so using the old entry pointer afterward looked wrong.
Clearly, something needed fixing there.
Looks like this could cause misses that could be hits regardless of
singlerow mode. With singlerow mode, the wrong entry could be marked
as complete, but that entry should already be either complete or be an
unused bucket. So it was just a case of not marking the current entry
as complete. When not in singlerow mode, some other unrelated entry
could be *unmarked* as complete, which again could result in misses
that should be hits.
The fix looks fine. It's a bit annoying that I went to the trouble of
coding cache_store_tuple() to go to the trouble of discovering when
the simplehash code shuffled cache entries around without actually
correctly using the newly discovered bucket :-(
Pushed.
David
Thank you for pushing the fix!
--
Best regards,
Ilia Evdokimov,
Tantor Labs LLC,
https://tantorlabs.com/
