windowagg_fix_v3.patch
text/plain
Filename: windowagg_fix_v3.patch
Type: text/plain
Part: 0
Message:
Re: Bug in row_number() optimization
Patch
Format: unified
Series: patch v3
| File | + | − |
|---|---|---|
| src/backend/executor/nodeWindowAgg.c | 20 | 0 |
| src/backend/optimizer/path/allpaths.c | 18 | 0 |
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 03ee6dc832..595870ea30 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -2399,6 +2399,24 @@ 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 the runcondition becomes false,
+ * we stop evaluating WindowFuncs in WindowAgg nodes and 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 for byref WindowFuncs and storing stale values for
+ * byval WindowFuncs (remember that window functions such as row_number()
+ * return a byref type 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 in the executor
+ * when the runcondition first becomes false.
+ */
+ set_opfuncid(opexpr);
+ if (!func_strict(opexpr->opfuncid))
+ return true;
+
/*
* 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