Re: Memoize: stale entry pointer after cache_store_tuple()

On 11/12/26 03:46, David Rowley wrote:

> On Thu, 12 Nov 2026 at 01:15, Ilia Evdokimov
> <ilya(dot)evdokimov(at)tantorlabs(dot)com> 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/