diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index d61d57e9a8..3b5f1526ed 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -41,6 +41,7 @@ #include "executor/nodeWindowAgg.h" #include "miscadmin.h" #include "nodes/nodeFuncs.h" +#include "optimizer/clauses.h" #include "optimizer/optimizer.h" #include "parser/parse_agg.h" #include "parser/parse_coerce.h" @@ -2803,16 +2804,25 @@ initialize_peragg(WindowAggState *winstate, WindowFunc *wfunc, * aggregate's arguments (and FILTER clause if any) contain any calls to * volatile functions. Otherwise, the difference between restarting and * not restarting the aggregation would be user-visible. + * Because contain_volatile_functions() does not recursively search + * subplans, we play it safe here and we don't use moving aggregates when + * we find subplans in the WindowFunc. For subplans existing in the + * WindowFunc's FILTER clause, it also seems like a good idea to not use + * moving aggregates even if the subplan didn't have a volatile function + * as there may be other reasons the subplan would produce a different + * result each time. For example, a syncscan with a LIMIT clause. */ if (!OidIsValid(aggform->aggminvtransfn)) use_ma_code = false; /* sine qua non */ else if (aggform->aggmfinalmodify == AGGMODIFY_READ_ONLY && - aggform->aggfinalmodify != AGGMODIFY_READ_ONLY) + aggform->aggfinalmodify != AGGMODIFY_READ_ONLY) use_ma_code = true; /* decision forced by safety */ else if (winstate->frameOptions & FRAMEOPTION_START_UNBOUNDED_PRECEDING) use_ma_code = false; /* non-moving frame head */ else if (contain_volatile_functions((Node *) wfunc)) use_ma_code = false; /* avoid possible behavioral change */ + else if (contain_subplans((Node *) wfunc)) + use_ma_code = false; /* subplans might contain volatile functions */ else use_ma_code = true; /* yes, let's use it */ if (use_ma_code)