Hi, On 2021-02-22 20:51:17 -0500, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > Except for the annoying issue that that we pervasively use Lists as > > expressions, I'd argue that we should actually cache "subtree > > volatility" in Expr nodes, similar to the way we use OpExpr.opfuncid > > etc. That'd allow us to make contain_volatile_functions() very cheap > > ... and completely break changing volatility with ALTER FUNCTION. > The case of OpExpr.opfuncid is okay only because we don't provide > a way to switch an operator's underlying function. (See also > 9f1255ac8.)
Hm. I was imagining we'd only set it within the planner. If so, I don't think it'd change anything around ALTER FUNCTION. But anyway, due to the List* issue, I don't think it's a viable approach as-is anyway. We could add a wrapper node around "planner expressions" that stores metadata about them during planning, without those properties leaking over expressions used at other times. E.g. having preprocess_expression() return a PlannerExpr that that points to the expression as preprocess_expression returns it today. That'd make it easy to cache information like volatility. But it also seems prohibitively invasive :(. > It'd certainly be desirable to reduce the number of duplicated > function property lookups in the planner, but I'm not convinced > that that is a good way to go about it. Do you have suggestions? Greetings, Andres Freund
