On Fri, 25 Nov 2022 at 00:52, Sergey Shinderuk
<s.shinde...@postgrespro.ru> wrote:
> Shouldn't we handle any pass-by-reference type the same? I suppose, a
> user-defined window function can return some other type, not int8.

Thanks for reporting this and to you and Richard for working on a fix.

I've just looked at it and it seems that valgrind is complaining
because a tuple formed by an upper-level WindowAgg contains a pointer
to free'd memory due to the byref type and eval_windowaggregates() not
having been executed to fill in ecxt_aggvalues and ecxt_aggnulls on
the lower-level WindowAgg.

Since upper-level WindowAggs cannot reference values calculated in
some lower-level WindowAgg, why can't we just NULLify the pointers
instead? See attached.

It is possible to have a monotonic window function that does not
return int8.  Technically something like MAX(text_col) OVER (PARTITION
BY somecol ORDER BY text_col) is monotonically increasing, it's just
that I didn't add a support function to tell the planner about that.
Someone could come along in the future and suggest we do that and show
us some convincing use case.  So whatever the fix, it cannot assume
the window function's return type is int8.

David
diff --git a/src/backend/executor/nodeWindowAgg.c 
b/src/backend/executor/nodeWindowAgg.c
index 81ba024bba..083a3b2386 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -2234,6 +2234,21 @@ ExecWindowAgg(PlanState *pstate)
                        if (winstate->numaggs > 0)
                                eval_windowaggregates(winstate);
                }
+               else if (!winstate->top_window)
+               {
+                       /*
+                        * Otherwise, if we're not the top-window, we'd better 
fill the
+                        * aggregate values in with something, else they might 
be pointing
+                        * to freed memory.  These don't need to be valid since 
WindowAggs
+                        * above us cannot reference the result of another 
WindowAgg.
+                        */
+                       numfuncs = winstate->numfuncs;
+                       for (i = 0; i < numfuncs; i++)
+                       {
+                               econtext->ecxt_aggvalues[i] = (Datum) 0;
+                               econtext->ecxt_aggnulls[i] = true;
+                       }
+               }
 
                /*
                 * If we have created auxiliary read pointers for the frame or 
group

Reply via email to