Re: Bug in row_number() optimization - Mailing list pgsql-hackers

From David Rowley
Subject Re: Bug in row_number() optimization
Date
Msg-id CAApHDvrZWEkoT_7Fu0b-rZfZvB9Rb8H8dzB4Exon+ozSgeyt-Q@mail.gmail.com
Whole thread Raw
In response to Re: Bug in row_number() optimization  (Sergey Shinderuk <s.shinderuk@postgrespro.ru>)
List pgsql-hackers
On Fri, 2 Dec 2022 at 00:21, Sergey Shinderuk
<s.shinderuk@postgrespro.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



pgsql-hackers by date:

Previous
From: "David G. Johnston"
Date:
Subject: Re: Optimize common expressions in projection evaluation
Next
From: Michael Paquier
Date:
Subject: Re: Add LSN along with offset to error messages reported for WAL file read/write/validate header failures