Hi,
For show_resultcache_info()
+ if (rcstate->shared_info != NULL)
+ {
The negated condition can be used with a return. This way, the loop can be unindented.
+ * ResultCache nodes are intended to sit above a parameterized node in the
+ * plan tree in order to cache results from them.
Since the parameterized node is singular, it would be nice if 'them' can be expanded to refer to the source of result cache.
+ rcstate->mem_used -= freed_mem;
Should there be assertion that after the subtraction, mem_used stays non-negative ?
+ if (found && entry->complete)
+ {
+ node->stats.cache_hits += 1; /* stats update */
Once inside the if block, we would return.
+ else
+ {
The else block can be unindented (dropping else keyword).
+ * return 1 row. XXX is this worth the check?
+ */
+ if (unlikely(entry->complete))
Since the check is on a flag (with minimal overhead), it seems the check can be kept, with the question removed.
Cheers
On Wed, 24 Mar 2021 at 00:42, David Rowley <dgrowleyml@gmail.com> wrote:
> I've now cleaned up the 0001 patch. I ended up changing a few places
> where we pass the RestrictInfo->clause to contain_volatile_functions()
> to instead pass the RestrictInfo itself so that there's a possibility
> of caching the volatility property for a subsequent call.
>
> I also made a pass over the remaining patches and for the 0004 patch,
> aside from the name, "Result Cache", I think that it's ready to go. We
> should consider before RC1 if we should have enable_resultcache switch
> on or off by default.
>
> Does anyone care to have a final look at these patches? I'd like to
> start pushing them fairly soon.
I've now pushed the 0001 patch to cache the volatility of PathTarget
and RestrictInfo.
I'll be looking at the remaining patches over the next few days.
Attached are a rebased set of patches on top of current master. The
only change is to the 0003 patch (was 0004) which had an unstable
regression test for parallel plan with a Result Cache. I've swapped
the unstable test for something that shouldn't fail randomly depending
on if a parallel worker did any work or not.
David
