From 6a716cd3dc1f808438dba393603ec955cae63e65 Mon Sep 17 00:00:00 2001 From: Henson Choi Date: Tue, 7 Apr 2026 13:59:29 +0900 Subject: [PATCH] Fix nav_slot pass-by-ref dangling pointer in RPR navigation When a DEFINE expression contains multiple navigation calls targeting different positions (e.g., PREV(x,1) > PREV(x,2)), the second call re-fetches nav_slot, freeing the previous tuple via pfree. Any pass-by-ref datum extracted from the first navigation becomes a dangling pointer. Fix by copying pass-by-ref results into per-tuple memory in the RESTORE step. --- src/backend/executor/execExpr.c | 5 ++ src/backend/executor/execExprInterp.c | 20 +++++++ src/include/executor/execExpr.h | 2 + src/test/regress/expected/rpr.out | 80 +++++++++++++++++++++++++++ src/test/regress/sql/rpr.sql | 34 ++++++++++++ 5 files changed, 141 insertions(+) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 6349a564a98..8d8da67e79f 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1304,7 +1304,12 @@ ExecInitExprRec(Expr *node, ExprState *state, /* Emit RESTORE opcode: restore original slot */ scratch.opcode = EEOP_RPR_NAV_RESTORE; + scratch.resvalue = resv; + scratch.resnull = resnull; scratch.d.rpr_nav.winstate = winstate; + get_typlenbyval(nav->resulttype, + &scratch.d.rpr_nav.resulttyplen, + &scratch.d.rpr_nav.resulttypbyval); ExprEvalPushStep(state, &scratch); break; } diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 2ec579732cc..e2d41c3098f 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -6156,6 +6156,13 @@ ExecEvalRPRNavSet(ExprState *state, ExprEvalStep *op, ExprContext *econtext) * When slot swap was elided (target == currentpos), this is a harmless * no-op since saved and current slots are identical. * The caller is responsible for updating any local slot cache. + * + * For pass-by-reference result types, the result datum points into + * nav_slot's tuple memory. If a subsequent navigation in the same + * expression re-fetches nav_slot for a different position, the old + * tuple is freed, leaving a dangling pointer. We prevent this by + * copying pass-by-ref results into per-tuple memory, which survives + * until the next ResetExprContext. */ void ExecEvalRPRNavRestore(ExprState *state, ExprEvalStep *op, @@ -6164,4 +6171,17 @@ ExecEvalRPRNavRestore(ExprState *state, ExprEvalStep *op, WindowAggState *winstate = op->d.rpr_nav.winstate; econtext->ecxt_outertuple = winstate->nav_saved_outertuple; + + /* Stabilize pass-by-ref result against nav_slot re-fetch */ + if (!op->d.rpr_nav.resulttypbyval && + !*op->resnull) + { + MemoryContext oldContext; + + oldContext = MemoryContextSwitchTo(econtext->ecxt_per_tuple_memory); + *op->resvalue = datumCopy(*op->resvalue, + false, + op->d.rpr_nav.resulttyplen); + MemoryContextSwitchTo(oldContext); + } } diff --git a/src/include/executor/execExpr.h b/src/include/executor/execExpr.h index 834800a4062..e6b2ab30406 100644 --- a/src/include/executor/execExpr.h +++ b/src/include/executor/execExpr.h @@ -703,6 +703,8 @@ typedef struct ExprEvalStep Datum *offset_value; /* offset value(s), or NULL */ bool *offset_isnull; /* offset null flag(s) */ /* For compound nav: offset_value[0] = inner, [1] = outer */ + int16 resulttyplen; /* RESTORE: result type length */ + bool resulttypbyval; /* RESTORE: result pass-by-value? */ } rpr_nav; /* for EEOP_AGG_*DESERIALIZE */ diff --git a/src/test/regress/expected/rpr.out b/src/test/regress/expected/rpr.out index 04ec25d4cf5..32aa8bc3722 100644 --- a/src/test/regress/expected/rpr.out +++ b/src/test/regress/expected/rpr.out @@ -1635,6 +1635,86 @@ WINDOW w AS ( company2 | 07-10-2023 | 1300 | | | 0 (20 rows) +-- Pass-by-ref types: two PREV calls targeting different positions. +-- Verifies that datumCopy in RESTORE prevents dangling pointers when +-- nav_slot is re-fetched for the second navigation. +-- tdate::text gives distinct text values per row (e.g. '07-01-2023'). +-- B matches when 1-back date text > 2-back date text (always true for +-- ascending dates), so B+ extends the full partition after A. +SELECT company, tdate, tdate::text AS tdate_text, + first_value(tdate::text) OVER w, last_value(tdate::text) OVER w, count(*) OVER w +FROM stock +WINDOW w AS ( + PARTITION BY company ORDER BY tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + AFTER MATCH SKIP PAST LAST ROW + PATTERN (A B+) + DEFINE + A AS TRUE, + B AS PREV(tdate::text, 1) > PREV(tdate::text, 2) +); + company | tdate | tdate_text | first_value | last_value | count +----------+------------+------------+-------------+------------+------- + company1 | 07-01-2023 | 07-01-2023 | | | 0 + company1 | 07-02-2023 | 07-02-2023 | 07-02-2023 | 07-10-2023 | 9 + company1 | 07-03-2023 | 07-03-2023 | | | 0 + company1 | 07-04-2023 | 07-04-2023 | | | 0 + company1 | 07-05-2023 | 07-05-2023 | | | 0 + company1 | 07-06-2023 | 07-06-2023 | | | 0 + company1 | 07-07-2023 | 07-07-2023 | | | 0 + company1 | 07-08-2023 | 07-08-2023 | | | 0 + company1 | 07-09-2023 | 07-09-2023 | | | 0 + company1 | 07-10-2023 | 07-10-2023 | | | 0 + company2 | 07-01-2023 | 07-01-2023 | | | 0 + company2 | 07-02-2023 | 07-02-2023 | 07-02-2023 | 07-10-2023 | 9 + company2 | 07-03-2023 | 07-03-2023 | | | 0 + company2 | 07-04-2023 | 07-04-2023 | | | 0 + company2 | 07-05-2023 | 07-05-2023 | | | 0 + company2 | 07-06-2023 | 07-06-2023 | | | 0 + company2 | 07-07-2023 | 07-07-2023 | | | 0 + company2 | 07-08-2023 | 07-08-2023 | | | 0 + company2 | 07-09-2023 | 07-09-2023 | | | 0 + company2 | 07-10-2023 | 07-10-2023 | | | 0 +(20 rows) + +-- numeric: PREV(price::numeric, 1) > PREV(price::numeric, 2) +-- B matches when price 1-back > price 2-back (ascending pair). +SELECT company, tdate, price::numeric AS nprice, + first_value(price::numeric) OVER w, last_value(price::numeric) OVER w, count(*) OVER w +FROM stock +WINDOW w AS ( + PARTITION BY company ORDER BY tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + AFTER MATCH SKIP PAST LAST ROW + PATTERN (A B+) + DEFINE + A AS TRUE, + B AS PREV(price::numeric, 1) > PREV(price::numeric, 2) +); + company | tdate | nprice | first_value | last_value | count +----------+------------+--------+-------------+------------+------- + company1 | 07-01-2023 | 100 | | | 0 + company1 | 07-02-2023 | 200 | 200 | 150 | 2 + company1 | 07-03-2023 | 150 | | | 0 + company1 | 07-04-2023 | 140 | | | 0 + company1 | 07-05-2023 | 150 | 150 | 90 | 2 + company1 | 07-06-2023 | 90 | | | 0 + company1 | 07-07-2023 | 110 | 110 | 120 | 3 + company1 | 07-08-2023 | 130 | | | 0 + company1 | 07-09-2023 | 120 | | | 0 + company1 | 07-10-2023 | 130 | | | 0 + company2 | 07-01-2023 | 50 | | | 0 + company2 | 07-02-2023 | 2000 | 2000 | 1500 | 2 + company2 | 07-03-2023 | 1500 | | | 0 + company2 | 07-04-2023 | 1400 | | | 0 + company2 | 07-05-2023 | 1500 | 1500 | 60 | 2 + company2 | 07-06-2023 | 60 | | | 0 + company2 | 07-07-2023 | 1100 | 1100 | 1200 | 3 + company2 | 07-08-2023 | 1300 | | | 0 + company2 | 07-09-2023 | 1200 | | | 0 + company2 | 07-10-2023 | 1300 | | | 0 +(20 rows) + -- -- FIRST/LAST navigation -- diff --git a/src/test/regress/sql/rpr.sql b/src/test/regress/sql/rpr.sql index a05b429ce74..724d460b2da 100644 --- a/src/test/regress/sql/rpr.sql +++ b/src/test/regress/sql/rpr.sql @@ -776,6 +776,40 @@ WINDOW w AS ( DEFINE A AS price > PREV(price, 1) AND price < NEXT(price, 1) ); +-- Pass-by-ref types: two PREV calls targeting different positions. +-- Verifies that datumCopy in RESTORE prevents dangling pointers when +-- nav_slot is re-fetched for the second navigation. +-- tdate::text gives distinct text values per row (e.g. '07-01-2023'). +-- B matches when 1-back date text > 2-back date text (always true for +-- ascending dates), so B+ extends the full partition after A. +SELECT company, tdate, tdate::text AS tdate_text, + first_value(tdate::text) OVER w, last_value(tdate::text) OVER w, count(*) OVER w +FROM stock +WINDOW w AS ( + PARTITION BY company ORDER BY tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + AFTER MATCH SKIP PAST LAST ROW + PATTERN (A B+) + DEFINE + A AS TRUE, + B AS PREV(tdate::text, 1) > PREV(tdate::text, 2) +); + +-- numeric: PREV(price::numeric, 1) > PREV(price::numeric, 2) +-- B matches when price 1-back > price 2-back (ascending pair). +SELECT company, tdate, price::numeric AS nprice, + first_value(price::numeric) OVER w, last_value(price::numeric) OVER w, count(*) OVER w +FROM stock +WINDOW w AS ( + PARTITION BY company ORDER BY tdate + ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + AFTER MATCH SKIP PAST LAST ROW + PATTERN (A B+) + DEFINE + A AS TRUE, + B AS PREV(price::numeric, 1) > PREV(price::numeric, 2) +); + -- -- FIRST/LAST navigation -- -- 2.50.1 (Apple Git-155)