From 8e806e30fd8a7dcbeb18e13faccd50cdeff0f911 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Fri, 5 Jun 2026 13:31:20 +0900 Subject: [PATCH 50/68] Fix signed-integer overflow in row pattern recognition frame-end clamp ExecRPRProcessRow() computed "frameOffset + 1" before passing it to pg_add_s64_overflow(), so a FOLLOWING offset near PG_INT64_MAX made that add overflow on its own -- undefined behavior that crashed cassert builds and gave wrong results otherwise. Add the offset and the +1 in two separately checked steps, clamping to PG_INT64_MAX on overflow, as core WindowAgg already does. The clamp is safe because the partition row count is an int64, so no row ever sits at that position; a huge FOLLOWING offset thus behaves like UNBOUNDED FOLLOWING. Also add a regression test. --- src/backend/executor/execRPR.c | 31 +++++++++++++++++++++----- src/test/regress/expected/rpr_base.out | 25 +++++++++++++++++++++ src/test/regress/sql/rpr_base.sql | 16 +++++++++++++ 3 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/backend/executor/execRPR.c b/src/backend/executor/execRPR.c index 56399c0c7fd..69e3603adef 100644 --- a/src/backend/executor/execRPR.c +++ b/src/backend/executor/execRPR.c @@ -1784,9 +1784,15 @@ ExecRPRProcessRow(WindowAggState *winstate, int64 currentPos, { int64 ctxFrameEnd; - /* Clamp to INT64_MAX on overflow */ - if (pg_add_s64_overflow(ctx->matchStartRow, frameOffset + 1, - &ctxFrameEnd)) + /* + * Clamp to INT64_MAX on overflow. frameOffset can be as large as + * PG_INT64_MAX (e.g. "ROWS FOLLOWING"), so add the offset + * and the trailing +1 in two separately checked steps to avoid + * signed-integer overflow in the "frameOffset + 1" subexpression. + */ + if (pg_add_s64_overflow(ctx->matchStartRow, frameOffset, + &ctxFrameEnd) || + pg_add_s64_overflow(ctxFrameEnd, 1, &ctxFrameEnd)) ctxFrameEnd = PG_INT64_MAX; /* @@ -1844,10 +1850,23 @@ ExecRPRProcessRow(WindowAggState *winstate, int64 currentPos, * mismatch (nfa_match with NULL), which removes all states (all * states are at VAR positions after advance). So any surviving * context here must be within its frame boundary. + * + * Compute the (clamped) frame end the same way as Phase 1, using two + * separately checked adds so that "frameOffset + 1" cannot overflow + * when frameOffset is near PG_INT64_MAX. */ - Assert(!hasLimitedFrame || - ctx->matchStartRow > PG_INT64_MAX - frameOffset - 1 || - currentPos < ctx->matchStartRow + frameOffset + 1); +#ifdef USE_ASSERT_CHECKING + if (hasLimitedFrame) + { + int64 ctxFrameEnd; + + if (pg_add_s64_overflow(ctx->matchStartRow, frameOffset, + &ctxFrameEnd) || + pg_add_s64_overflow(ctxFrameEnd, 1, &ctxFrameEnd)) + ctxFrameEnd = PG_INT64_MAX; + Assert(currentPos < ctxFrameEnd); + } +#endif nfa_advance(winstate, ctx, currentPos); } diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out index 1410ba75395..4fe7360114f 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -643,6 +643,31 @@ ORDER BY id; 6 | 30 | 1 (6 rows) +-- int64 frame-end overflow: a huge FOLLOWING offset must clamp to the +-- partition end (matchStartRow + offset + 1 overflows int64; the clamp makes +-- it behave like UNBOUNDED FOLLOWING). Guards against signed-integer overflow +-- in the "frameOffset + 1" subexpression (undefined behavior). The cnt values +-- must match the UNBOUNDED FOLLOWING result for the same data. +SELECT id, val, COUNT(*) OVER w as cnt +FROM rpr_frame +WINDOW w AS ( + ORDER BY id + ROWS BETWEEN CURRENT ROW AND 9223372036854775806 FOLLOWING + AFTER MATCH SKIP TO NEXT ROW + PATTERN (A+) + DEFINE A AS val > 0 +) +ORDER BY id; + id | val | cnt +----+-----+----- + 1 | 10 | 6 + 2 | 10 | 5 + 3 | 10 | 4 + 4 | 20 | 3 + 5 | 20 | 2 + 6 | 30 | 1 +(6 rows) + -- RANGE frame with RPR (not permitted) SELECT id, val, COUNT(*) OVER w as cnt FROM rpr_frame diff --git a/src/test/regress/sql/rpr_base.sql b/src/test/regress/sql/rpr_base.sql index 53bf090b903..c6fcfa3e9ff 100644 --- a/src/test/regress/sql/rpr_base.sql +++ b/src/test/regress/sql/rpr_base.sql @@ -515,6 +515,22 @@ WINDOW w AS ( ) ORDER BY id; +-- int64 frame-end overflow: a huge FOLLOWING offset must clamp to the +-- partition end (matchStartRow + offset + 1 overflows int64; the clamp makes +-- it behave like UNBOUNDED FOLLOWING). Guards against signed-integer overflow +-- in the "frameOffset + 1" subexpression (undefined behavior). The cnt values +-- must match the UNBOUNDED FOLLOWING result for the same data. +SELECT id, val, COUNT(*) OVER w as cnt +FROM rpr_frame +WINDOW w AS ( + ORDER BY id + ROWS BETWEEN CURRENT ROW AND 9223372036854775806 FOLLOWING + AFTER MATCH SKIP TO NEXT ROW + PATTERN (A+) + DEFINE A AS val > 0 +) +ORDER BY id; + -- RANGE frame with RPR (not permitted) SELECT id, val, COUNT(*) OVER w as cnt FROM rpr_frame -- 2.50.1 (Apple Git-155)