Hi, On 2021-02-16 23:15:51 +1300, David Rowley wrote: > There might be gains to be had by checking the parse once rather than > having to call contains_volatile_functions in the various places we do > call it. I think both the parallel safety and volatile checks could > then be done in the same tree traverse. Anyway. I've not done any > hacking on this. It's just an idea so far.
ISTM that it could be worth to that as part of preprocess_expression() - it's a pass that we unconditionally do pretty early, it already computes opfuncid, often already fetches the pg_proc entry (cf simplify_function()), etc. 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 in the majority of cases, but we could still easily invalidate that state when necessary by setting "exprhasvolatile" to unknown (causing the next contain_volatile_functions() to compute it from scratch). But since we actually do use Lists as expressions (which do not inherit from Expr), we'd instead need to pass a new param to preprocess_expression() that stores the volatility somewhere in PlannerInfo? Seems a bit yucky to manage :(. Greetings, Andres Freund
