Re: Bug in row_number() optimization

On Fri, 2 Dec 2022 at 00:21, Sergey Shinderuk
<s(dot)shinderuk(at)postgrespro(dot)ru> wrote:
> Maybe I'm missing something, but the previous call to spool_tuples()
> might have read extra tuples (if the tuplestore spilled to disk), and
> after switching to WINDOWAGG_PASSTHROUGH_STRICT mode we nevertheless
> would loop through these extra tuples and call ExecProject if only to
> increment winstate->currentpos.

The tuples which are spooled in the WindowAgg node are the ones from
the WindowAgg's subnode. Since these don't contain the results of the
WindowFunc, then I don't think there's any issue with what's stored in
any of the spooled tuples.

What matters is what we pass along to the node that's reading from the
WindowAgg. If we NULL out the memory where we store the WindowFunc
(and maybe an Aggref) results then the ExecProject in ExecWindowAgg()
will no longer fill the WindowAgg's output slot with the address of
free'd memory (or some stale byval value which has lingered for byval
return type WindowFuncs).

Since the patch I sent sets the context's ecxt_aggnulls to true, it
means that when we do the ExecProject(), the EEOP_WINDOW_FUNC in
ExecInterpExpr (or the JIT equivalent) will put an SQL NULL in the
*output* slot for the WindowAgg node. The same is true for
EEOP_AGGREFs as the WindowAgg node that we are running in
WINDOWAGG_PASSTHROUGH mode could also contain normal aggregate
functions, not just WindowFuncs.

David