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