On Sat, 26 Nov 2022 at 05:19, Tom Lane <[email protected]> wrote:
>
> Sergey Shinderuk <[email protected]> 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
