David Rowley <dgrowle...@gmail.com> writes: > On Tue, 23 Feb 2021 at 18:43, Tom Lane <t...@sss.pgh.pa.us> wrote: >> I doubt it's that bad. We could cache such info in RestrictInfo >> for quals, or PathTarget for tlists, without much new notational >> overhead. That doesn't cover everything the planner deals with >> of course, but it would cover enough that you'd be chasing pretty >> small returns to worry about more.
> This seems like a pretty good idea. So I coded it up. > The 0001 patch adds a has_volatile bool field to RestrictInfo and sets > it when building the RestrictInfo. 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. regards, tom lane