On Mon, 15 Mar 2021 at 23:57, David Rowley <dgrowleyml@gmail.com> wrote:
>
> On Fri, 12 Mar 2021 at 14:59, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > I'm -1 on doing it exactly that way, because you're expending
> > the cost of those lookups without certainty that you need the answer.
> > I had in mind something more like the way that we cache selectivity
> > estimates in RestrictInfo, in which the value is cached when first
> > demanded and then re-used on subsequent checks --- see in
> > clause_selectivity_ext, around line 750. You do need a way for the
> > field to have a "not known yet" value, but that's not hard. Moreover,
> > this sort of approach can be less invasive than what you did here,
> > because the caching behavior can be hidden inside
> > contain_volatile_functions, rather than having all the call sites
> > know about it explicitly.
>
> I coded up something more along the lines of what I think you had in
> mind for the 0001 patch.
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.
David
