From b228235116352a38476a5d33c1db3ccb59ccb5b8 Mon Sep 17 00:00:00 2001 From: jian he Date: Mon, 10 Aug 2026 09:29:38 +0900 Subject: [PATCH] Return null from a row pattern navigation to a nonexistent row When the target row does not exist, ExecRPRNavGetSlot() handed back an all-NULL tuple and the argument expression was evaluated against it, so a missing row behaved like a row of nulls: PREV(val IS NOT NULL) on the first row of a partition returned false rather than null. ISO/IEC TR 19075-5 5.6.2 has the navigation itself return the null value when there is no such row. ExecEvalRPRNavSet() now reports through resnull whether the target row exists, and an EEOP_JUMP_IF_NULL step after EEOP_RPR_NAV_SET branches straight to RESTORE, so the argument is never evaluated. resnull is written on every path, including the slot swap elision one, since it may still hold a value from an earlier evaluation. The skip is data-dependent: an error inside the argument, such as a division by zero, is raised only on the rows whose target row exists. Constant folding still reaches into the argument and evaluates it while planning, putting the current row's value where the target row's belongs, so the property above holds at run time only. A folded value is the same on every row, so no answer changes, but a division by zero is raised for a row the navigation could never have reached. A separate patch will deal with the folding; the test marks the query that shows it with XXX. The all-NULL tuple is therefore no longer load-bearing. Remove WindowAggState.nav_null_slot, its initialization, and the TupleTableSlot every WindowAgg with a PATTERN clause allocated in es_tupleTable; ExecRPRNavGetSlot() returns NULL instead. --- src/backend/executor/execExpr.c | 18 +++ src/backend/executor/execExprInterp.c | 20 ++- src/backend/executor/nodeWindowAgg.c | 10 +- src/include/nodes/execnodes.h | 1 - src/test/regress/expected/rpr_base.out | 190 +++++++++++++++++++++++++ src/test/regress/sql/rpr_base.sql | 100 +++++++++++++ 6 files changed, 330 insertions(+), 9 deletions(-) diff --git a/src/backend/executor/execExpr.c b/src/backend/executor/execExpr.c index 8e812fdcdc5..7a79a002111 100644 --- a/src/backend/executor/execExpr.c +++ b/src/backend/executor/execExpr.c @@ -1184,6 +1184,7 @@ ExecInitExprRec(Expr *node, ExprState *state, */ RPRNavExpr *nav = (RPRNavExpr *) node; WindowAggState *winstate; + int skip_arg_step; Assert(state->parent && IsA(state->parent, WindowAggState)); winstate = (WindowAggState *) state->parent; @@ -1245,9 +1246,26 @@ ExecInitExprRec(Expr *node, ExprState *state, ExprEvalPushStep(state, &scratch); + /* + * If the target row does not exist, skip evaluation of the + * argument expression and go straight to RESTORE. The + * EEOP_RPR_NAV_SET step writes a definitive resnull (false + * when the target row exists), so the jump condition is + * always up to date. + */ + skip_arg_step = state->steps_len; + scratch.opcode = EEOP_JUMP_IF_NULL; + scratch.resvalue = resv; + scratch.resnull = resnull; + scratch.d.jump.jumpdone = -1; /* set below */ + ExprEvalPushStep(state, &scratch); + /* Compile the argument expression normally */ ExecInitExprRec(nav->arg, state, resv, resnull); + /* out-of-range jump lands on the RESTORE step */ + state->steps[skip_arg_step].d.jump.jumpdone = state->steps_len; + /* Emit RESTORE opcode: restore original slot */ scratch.opcode = EEOP_RPR_NAV_RESTORE; scratch.resvalue = resv; diff --git a/src/backend/executor/execExprInterp.c b/src/backend/executor/execExprInterp.c index 7ace95fe0a9..c7460fbfac9 100644 --- a/src/backend/executor/execExprInterp.c +++ b/src/backend/executor/execExprInterp.c @@ -6219,11 +6219,29 @@ ExecEvalRPRNavSet(ExprState *state, ExprEvalStep *op, ExprContext *econtext) * EEOP_RPR_NAV_RESTORE is a harmless no-op. */ if (target_pos == winstate->currentpos) + { + /* target row trivially exists; see comment below */ + *op->resnull = false; return; + } - /* Fetch target row slot (returns nav_null_slot if out of range) */ target_slot = ExecRPRNavGetSlot(winstate, target_pos); + /* + * Report whether the target row exists through resnull, which the jump + * step tests before the argument expression gets to overwrite it: null + * when the row is out of range, so the jump skips the argument, and a + * definitive false otherwise, since resnull may still hold a stale value + * from a previous evaluation. + */ + if (target_slot == NULL) + { + *op->resvalue = (Datum) 0; + *op->resnull = true; + return; + } + *op->resnull = false; + /* * Update econtext to point to the target slot. Also decompress the new * slot's attributes since FETCHSOME already ran for the original slot. diff --git a/src/backend/executor/nodeWindowAgg.c b/src/backend/executor/nodeWindowAgg.c index 0828fdd5f8c..972874b3514 100644 --- a/src/backend/executor/nodeWindowAgg.c +++ b/src/backend/executor/nodeWindowAgg.c @@ -2822,10 +2822,6 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) &TTSOpsMinimalTuple); winstate->nav_slot_pos = -1; - winstate->nav_null_slot = ExecInitExtraTupleSlot(estate, scanDesc, - &TTSOpsMinimalTuple); - winstate->nav_null_slot = ExecStoreAllNullTuple(winstate->nav_null_slot); - winstate->nav_saved_outertuple = NULL; winstate->nav_match_start = 0; } @@ -3111,7 +3107,7 @@ ExecInitWindowAgg(WindowAgg *node, EState *estate, int eflags) * ExecRPRNavGetSlot * * Fetch tuple at given position for RPR navigation opcodes. - * Returns nav_slot with the tuple loaded, or nav_null_slot if out of range. + * Returns nav_slot with the tuple loaded, or NULL if out of range. */ TupleTableSlot * ExecRPRNavGetSlot(WindowAggState *winstate, int64 pos) @@ -3120,7 +3116,7 @@ ExecRPRNavGetSlot(WindowAggState *winstate, int64 pos) TupleTableSlot *slot = winstate->nav_slot; if (pos < 0) - return winstate->nav_null_slot; + return NULL; /* * If nav_slot already holds this position, return it without re-fetching. @@ -3135,7 +3131,7 @@ ExecRPRNavGetSlot(WindowAggState *winstate, int64 pos) if (!window_gettupleslot(winobj, pos, slot)) { winstate->nav_slot_pos = -1; - return winstate->nav_null_slot; + return NULL; } winstate->nav_slot_pos = pos; diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index aab2aa575d4..01c2355b576 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -2751,7 +2751,6 @@ typedef struct WindowAggState int64 nav_slot_pos; /* position cached in nav_slot, or -1 */ TupleTableSlot *nav_slot; /* slot for PREV/NEXT/FIRST/LAST target row */ TupleTableSlot *nav_saved_outertuple; /* saved slot during nav swap */ - TupleTableSlot *nav_null_slot; /* all NULL slot */ int64 nav_match_start; /* match_start for FIRST/LAST nav */ /* RPR current match result */ diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out index 54fa490a4e1..6033272d1ce 100644 --- a/src/test/regress/expected/rpr_base.out +++ b/src/test/regress/expected/rpr_base.out @@ -1746,6 +1746,196 @@ DETAIL: Pattern has 32768 elements, maximum is 32767. CREATE TABLE rpr_nav (id INT, val INT); INSERT INTO rpr_nav VALUES (1, 10), (2, 20), (3, 15), (4, 25), (5, 30); +-- Avoid evaluating the inner argument expression when the target row is out +-- of range or does not exist. PREV misses on the first row of the partition +-- and NEXT on the last, so the two directions are checked separately. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(val is not null) is null); + id | cnt +----+----- + 1 | 1 + 2 | 0 + 3 | 0 + 4 | 0 + 5 | 0 +(5 rows) + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS NEXT(val is not null) is null); + id | cnt +----+----- + 1 | 0 + 2 | 0 + 3 | 0 + 4 | 0 + 5 | 1 +(5 rows) + +-- FIRST and LAST cannot miss under PATTERN (A): the match is one row long, so +-- the target is the current row and the slot swap is elided. These two run +-- that path; the query further down is what pins what it writes. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS LAST(val is not null) is null); + id | cnt +----+----- + 1 | 0 + 2 | 0 + 3 | 0 + 4 | 0 + 5 | 0 +(5 rows) + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS FIRST(val is not null) is null); + id | cnt +----+----- + 1 | 0 + 2 | 0 + 3 | 0 + 4 | 0 + 5 | 0 +(5 rows) + +-- The elided path has to report the target row as present, since resnull may +-- still be null from a scan whose navigation missed. Here the navigation is +-- the whole DEFINE, so nothing overwrites resnull in between, and inlining the +-- function makes the offset a parameter: 1 misses, then 0 elides. +CREATE FUNCTION rpr_nav_off(k int) RETURNS SETOF bigint LANGUAGE sql STABLE AS $$ + SELECT count(*) OVER w FROM rpr_nav WHERE id = 1 + WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A) DEFINE A AS PREV(val > 0, k)) $$; +SELECT o.k, f FROM (VALUES (1), (0)) o(k), LATERAL rpr_nav_off(o.k) f; + k | f +---+--- + 1 | 0 + 0 | 1 +(2 rows) + +DROP FUNCTION rpr_nav_off(int); +-- Under a longer pattern FIRST and LAST can miss. On the first row of a +-- match currentpos equals match_start, so an offset of one falls outside the +-- match in either direction, and the compound forms miss on their inner +-- navigation, which is a separate bound from the one the plain forms cross. +-- The argument does not propagate null, so a missing row is told apart from +-- a row of nulls. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) DEFINE A AS LAST(val is not null, 1) is null); + id | cnt +----+----- + 1 | 1 + 2 | 1 + 3 | 1 + 4 | 1 + 5 | 1 +(5 rows) + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) DEFINE A AS FIRST(val is not null, 1) is null); + id | cnt +----+----- + 1 | 1 + 2 | 1 + 3 | 1 + 4 | 1 + 5 | 1 +(5 rows) + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) DEFINE A AS PREV(FIRST(val is not null, 1), 2) is null); + id | cnt +----+----- + 1 | 5 + 2 | 0 + 3 | 0 + 4 | 0 + 5 | 0 +(5 rows) + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) DEFINE A AS PREV(LAST(val is not null, 1), 2) is null); + id | cnt +----+----- + 1 | 3 + 2 | 0 + 3 | 0 + 4 | 1 + 5 | 1 +(5 rows) + +-- Not a duplicate of the PREV(val is not null) case: the argument here is +-- true where that one is false, and a missing row still has to win over both. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(val is null) is null); + id | cnt +----+----- + 1 | 1 + 2 | 0 + 3 | 0 + 4 | 0 + 5 | 0 +(5 rows) + +-- Skipping the argument is data-dependent: restricted to the row PREV misses +-- on, a division by zero in it never runs; over the whole table NEXT reaches +-- a row for all but the last and it does. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t WHERE id = 1 +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(val / 0) > 0); + id | cnt +----+----- + 1 | 0 +(1 row) + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS NEXT(val / 0) > 0); +ERROR: division by zero +-- Here the null reaches the DEFINE predicate itself instead of an IS NULL +-- test: an all-NULL target row would have made v IS NULL true and matched the +-- first row, so this pins the predicate side of the same behaviour. +WITH t(id, v) AS (VALUES (1, 10), (2, 20)) +SELECT id, count(*) OVER w AS cnt +FROM t +WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(v IS NULL)); + id | cnt +----+----- + 1 | 0 + 2 | 0 +(2 rows) + +-- Constant folding can leave a navigation argument with no column reference +-- at all (v folds to 10, so PREV(v IS NULL) becomes PREV(false)), which the +-- planner has to accept rather than re-run the parse-time rejection. +WITH t(id, v) AS (VALUES (1, 10)) +SELECT id, count(*) OVER w AS cnt +FROM t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(v IS NULL)); + id | cnt +----+----- + 1 | 0 +(1 row) + +-- XXX Folding evaluates the argument while planning, with the current row's +-- value standing in for the target row's, so this divides by zero even though +-- PREV has no row to navigate to. A separate patch will deal with it. +WITH t(id, v) AS (VALUES (1, 10)) +SELECT id, count(*) OVER w AS cnt +FROM t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(v / 0) > 0); +ERROR: division by zero -- PREV function - reference previous row in pattern SELECT id, val, COUNT(*) OVER w as cnt FROM rpr_nav diff --git a/src/test/regress/sql/rpr_base.sql b/src/test/regress/sql/rpr_base.sql index 33856187d62..4842581b859 100644 --- a/src/test/regress/sql/rpr_base.sql +++ b/src/test/regress/sql/rpr_base.sql @@ -1290,6 +1290,106 @@ CREATE TABLE rpr_nav (id INT, val INT); INSERT INTO rpr_nav VALUES (1, 10), (2, 20), (3, 15), (4, 25), (5, 30); +-- Avoid evaluating the inner argument expression when the target row is out +-- of range or does not exist. PREV misses on the first row of the partition +-- and NEXT on the last, so the two directions are checked separately. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(val is not null) is null); + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS NEXT(val is not null) is null); + +-- FIRST and LAST cannot miss under PATTERN (A): the match is one row long, so +-- the target is the current row and the slot swap is elided. These two run +-- that path; the query further down is what pins what it writes. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS LAST(val is not null) is null); + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS FIRST(val is not null) is null); + +-- The elided path has to report the target row as present, since resnull may +-- still be null from a scan whose navigation missed. Here the navigation is +-- the whole DEFINE, so nothing overwrites resnull in between, and inlining the +-- function makes the offset a parameter: 1 misses, then 0 elides. +CREATE FUNCTION rpr_nav_off(k int) RETURNS SETOF bigint LANGUAGE sql STABLE AS $$ + SELECT count(*) OVER w FROM rpr_nav WHERE id = 1 + WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A) DEFINE A AS PREV(val > 0, k)) $$; +SELECT o.k, f FROM (VALUES (1), (0)) o(k), LATERAL rpr_nav_off(o.k) f; +DROP FUNCTION rpr_nav_off(int); + +-- Under a longer pattern FIRST and LAST can miss. On the first row of a +-- match currentpos equals match_start, so an offset of one falls outside the +-- match in either direction, and the compound forms miss on their inner +-- navigation, which is a separate bound from the one the plain forms cross. +-- The argument does not propagate null, so a missing row is told apart from +-- a row of nulls. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) DEFINE A AS LAST(val is not null, 1) is null); + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) DEFINE A AS FIRST(val is not null, 1) is null); + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) DEFINE A AS PREV(FIRST(val is not null, 1), 2) is null); + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING + PATTERN (A+) DEFINE A AS PREV(LAST(val is not null, 1), 2) is null); + +-- Not a duplicate of the PREV(val is not null) case: the argument here is +-- true where that one is false, and a missing row still has to win over both. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(val is null) is null); + +-- Skipping the argument is data-dependent: restricted to the row PREV misses +-- on, a division by zero in it never runs; over the whole table NEXT reaches +-- a row for all but the last and it does. +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t WHERE id = 1 +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(val / 0) > 0); + +SELECT id, count(*) OVER w AS cnt +FROM rpr_nav t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS NEXT(val / 0) > 0); + +-- Here the null reaches the DEFINE predicate itself instead of an IS NULL +-- test: an all-NULL target row would have made v IS NULL true and matched the +-- first row, so this pins the predicate side of the same behaviour. +WITH t(id, v) AS (VALUES (1, 10), (2, 20)) +SELECT id, count(*) OVER w AS cnt +FROM t +WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(v IS NULL)); + +-- Constant folding can leave a navigation argument with no column reference +-- at all (v folds to 10, so PREV(v IS NULL) becomes PREV(false)), which the +-- planner has to accept rather than re-run the parse-time rejection. +WITH t(id, v) AS (VALUES (1, 10)) +SELECT id, count(*) OVER w AS cnt +FROM t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(v IS NULL)); + +-- XXX Folding evaluates the argument while planning, with the current row's +-- value standing in for the target row's, so this divides by zero even though +-- PREV has no row to navigate to. A separate patch will deal with it. +WITH t(id, v) AS (VALUES (1, 10)) +SELECT id, count(*) OVER w AS cnt +FROM t +WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING PATTERN (A) DEFINE A AS PREV(v / 0) > 0); + -- PREV function - reference previous row in pattern SELECT id, val, COUNT(*) OVER w as cnt FROM rpr_nav