Hi Jian, Tatsuo, > based on https://github.com/assam258-5892/postgres/tree/RPR-20260930 > I have made some non-trivial refactoring in nodeWindowAgg.c, see attached.
Thank you for the refactoring. Reading it, I came to think that for the frame we should rethink the design before splitting the code. There are two reasons. - SEEK. Under SEEK a match may start after the current row. Today the reduced frame is laid over the frame on the assumption that it always starts at the current row, so there is no place to express that case. - Inverse transition functions. Today the aggregates of an RPR window are recomputed from scratch for every row, even under SKIP TO NEXT ROW where the frames of neighboring rows mostly overlap, so the time grows with the square of the number of rows. While the frames overlap, the aggregates should be carried forward with the inverse transition function, as in an ordinary window. Both come down to how the reduced frame is represented. The standard (ISO/IEC 19075-5, 6.10.1) defines the window frame as a subtractive process. Starting from the partition, rows are removed by the frame extent and then by EXCLUDE, which gives the full window frame. With row pattern recognition there is one more step: only the rows mapped by the match remain, and that is the reduced window frame. So instead of laying the reduced frame over the frame and checking it separately, we could express it as the frame itself, by narrowing the frame's start and end to the match. For a window without RPR the last step does nothing, so the reduced frame is simply the frame we have today. Window functions and aggregates would then only look at the frame, hardly needing to know about RPR, and the RPR handling now spread over frame access and aggregation would gather in the one place that sets the frame positions plus a few short checks. This settles both reasons. The core already handles frames that do not start at the current row (ROWS BETWEEN 1 FOLLOWING AND 3 FOLLOWING) and empty frames. A match under SEEK can be handled like a frame starting n rows after the current row, only with n differing from row to row. The aggregates take the ordinary moving-aggregate path. The only new property of the reduced frame is that its end can move backward, for example when the next match is shorter under SKIP TO NEXT ROW, and only then do they need a restart. Part of your patch already goes this way. Driving the match only from ExecWindowAgg() and having everything else read the recorded result is what this design needs first, since the current row's match has to be resolved before any frame position is read, and making WinGetFuncArgInFrame() the same as in master is where this design is heading too. The separate RPR accessor, however, is not needed in this design and would have to be changed again anyway. So rather than add it with its duplication, how about taking enough time and replacing that part with this design? Best regards, Henson
