On Tue, Jul 28, 2026 at 10:48 AM Tatsuo Ishii <[email protected]> wrote:
>
> Thanks to Jian He for an extensive review that helped surface several
> defects and substantially improved the readability and maintainability
> of the code; the review also prompted the redesign of the
> window-navigation functions.
>
> Discussion:
> https://postgr.es/m/20230625.210509.1276733411677577841.t-ishii%40sranhm.sra.co.jp
HI.
In src/backend/executor/nodeWindowAgg.c, we have:
```
if (rpr_is_defined(winstate))
{
if (winstate->rpSkipTo == ST_NEXT_ROW)
clear_reduced_frame(winstate);
/*
* Drive the row pattern match every row, so it tracks the row
* scan rather than frame access: a window function that skips
* the frame (e.g. nth_value() with a NULL offset) must not
* leave the match state behind currentpos.
*/
Assert(winstate->nav_winobj != NULL);
ensure_reduced_frame(winstate->nav_winobj,
winstate->currentpos);
}
```
and
ensure_reduced_frame(WindowObject winobj, int64 pos)
In the above ``winstate->currentpos`` clearly is more readable than "pos",
ensure_reduced_frame only used twice.
So I propose removing both ensure_reduced_frame and
clear_reduced_frame, and inlining their corresponding function
contents at the call sites.
given the whole code change in nodeWindowAgg.c is quite complicated,
these readability changes are necessary, IMHO.
--
jian
https://www.enterprisedb.com/
