On Sat, 26 Nov 2022 at 05:19, Tom Lane <t...@sss.pgh.pa.us> wrote: > > Sergey Shinderuk <s.shinde...@postgrespro.ru> writes: > > What about user-defined operators? I created my own <= operator for int8 > > which returns true on null input, and put it in a btree operator class. > > Admittedly, it's weird that (null <= 1) evaluates to true. But does it > > violate the contract of the btree operator class or something? Didn't > > find a clear answer in the docs. > > It's pretty unlikely that this would work during an actual index scan. > I'm fairly sure that btree (and other index AMs) have hard-wired > assumptions that index operators are strict.
If we're worried about that then we could just restrict this optimization to only work with strict quals. The proposal to copy the datums into the query context does not seem to me to be a good idea. If there are a large number of partitions then it sounds like we'll leak lots of memory. We could invent some partition context that we reset after each partition, but that's probably more complexity than it would be worth. I've attached a draft patch to move the code to nullify the aggregate results so that's only done once per partition and adjusted the planner to limit this to strict quals. David
diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 81ba024bba..110c594edd 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -2300,7 +2300,27 @@ ExecWindowAgg(PlanState *pstate) continue; } else + { winstate->status = WINDOWAGG_PASSTHROUGH; + + /* + * Otherwise, if we're not the top-window, we'd better + * NULLify the aggregate results, else, by-ref result + * types will point to freed memory. We can get away + * without storing the final result of the WindowFunc + * because in the planner we insisted that the + * runcondition is strict. Setting these to NULL will + * ensure the top-level WindowAgg filter will always + * filter out the rows produced in this WindowAgg when + * not in WINDOWAGG_RUN state. + */ + numfuncs = winstate->numfuncs; + for (i = 0; i < numfuncs; i++) + { + econtext->ecxt_aggvalues[i] = (Datum) 0; + econtext->ecxt_aggnulls[i] = true; + } + } } else { diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c index 4ddaed31a4..a25956200e 100644 --- a/src/backend/optimizer/path/allpaths.c +++ b/src/backend/optimizer/path/allpaths.c @@ -2399,6 +2399,22 @@ check_and_push_window_quals(Query *subquery, RangeTblEntry *rte, Index rti, if (list_length(opexpr->args) != 2) return true; + /* + * Currently, we restrict this optimization to strict OpExprs. The reason + * for this is that during execution, once we stop evaluating WindowFuncs + * on lower-level WindowAgg nodes, since we're no longer updating them, + * we NULLify the aggregate and window aggregate results to prevent by-ref + * result typed window aggregates from pointing to free'd memory (this can + * happen on non-FLOAT8PASSBYVAL builds). Upper-level WindowAgg nodes may + * make reference to these results in their filter clause and we can + * ensure the filter remain false by making sure the operator is strict + * and by performing the NULLification when the run-condition is no longer + * true. + */ + set_opfuncid(opexpr); + if (!func_strict(opexpr->opfuncid)) + return false; + /* * Check for plain Vars that reference window functions in the subquery. * If we find any, we'll ask find_window_run_conditions() if 'opexpr' can