Hi Tatsuo, Jian,
Two problems, and they came up together: the second is what let me
write the second reproduction of the first.
1. A DEFINE clause can take the backend down. Patch attached.
2. A DEFINE can name what looks like a volatile function, when the
same expression is in GROUP BY. I read that as the existing
conventions working rather than as a hole, but I would like to
hear if anyone sees it differently.
Setup for everything below:
CREATE TABLE t (id int, val int);
INSERT INTO t VALUES (1, 10), (2, 20), (3, 15), (4, 30), (5, 5);
1. The crash
A navigation offset spelled the same as a window ORDER BY key:
SELECT id, val, count(*) OVER w AS cnt
FROM t
WINDOW w AS (
ORDER BY (extract(hour from localtimestamp)::int * 0 + 1), id
ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
PATTERN (A B+)
DEFINE B AS val >
PREV(val, (extract(hour from localtimestamp)::int * 0 + 1)));
server closed the connection unexpectedly
And spelled the same as a GROUP BY expression:
SELECT id, val, count(*) OVER w AS cnt
FROM t
GROUP BY GROUPING SETS ((id, val, ((random() * 0)::bigint + 1)),
(id, ((random() * 0)::bigint + 1)))
WINDOW w AS (
ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
PATTERN (A B+)
DEFINE B AS val > PREV(val, (random() * 0)::bigint + 1))
ORDER BY id, val;
server closed the connection unexpectedly
On a cassert + ASan build both abort with signal 6, frame for frame
the same:
#0 CheckVarSlotCompatibility execExprInterp.c:2431
#1 CheckExprStillValid
#2 ExecInterpExprStillValid execExprInterp.c:2325
#3 ExecEvalExprSwitchContext executor.h:452
#4 eval_nav_offset nodeWindowAgg.c:4092
#5 resolve_one_nav nodeWindowAgg.c:4291
#6 resolve_nav_offsets nodeWindowAgg.c:4422
#7 ExecWindowAgg nodeWindowAgg.c:2473
That line reads slot->tts_tupleDescriptor. The slot is null: UBSan
calls it a member access within a null pointer, and ASan reports the
read at 0x10, which is that member's offset. Nothing is corrupted --
the slot simply is not set yet.
set_upper_references() hands the whole DEFINE expression to
fix_upper_expr(), which matches any subexpression against the
subplan's targetlist. An offset spelled the same as something already
in the window input is therefore replaced with a Var(OUTER_VAR)
referencing it, and the compiled expression gets an EEOP_OUTER_VAR
step. resolve_nav_offsets() then evaluates the offsets at the top of
ExecWindowAgg(), before the partition is opened and before
ecxt_outertuple has been pointed at anything.
The substitution does not get the value wrong -- the window input
holds the result of that same expression -- it gets the timing wrong.
An offset is resolved once per scan; a Var is a per-row read.
Two things have to line up, and it is worth saying which:
- the offset must be equal() to something in the window input, which
is why both reproductions spell it twice. Change one of the two and
the query runs;
- it must not be a Const, because
search_indexed_tlist_for_non_var() declines to match one
("replacing it with a Var is silly"). That is why the offset has to
survive constant folding.
The "* 0" is not part of either condition. It only pins the value so
the regression output does not move; without it the crash is the same.
The fix. Only the navigated argument is read from the input, one row
at a time; the offsets are run-time constants. set_plan_refs()
already draws that line for the frame offsets of the same node, a few
lines above the DEFINE block:
* Like Limit node limit/offset expressions, WindowAgg has
* frame offset expressions, which cannot contain subplan
* variable refs, so fix_scan_expr works for them.
So RPRNavExpr gets its own case in fix_upper_expr_mutator(): arg
recurses as before, the two offsets go to fix_scan_expr(). That is
the whole change, 22 lines in setrefs.c.
Both reproductions and a control are now in rpr_base. With the case
taken back out they abort again, so they do hold the fix in place.
All five RPR suites pass, with no sanitizer output.
2. Why the second reproduction is accepted at all
DEFINE rejects volatile functions:
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 val > (random() * 0)::int);
ERROR: DEFINE clause cannot contain volatile functions
Name the same expression in GROUP BY and the same DEFINE is accepted:
SELECT id, count(*) OVER w AS cnt
FROM t
GROUP BY id, val, ((random() * 0)::int)
WINDOW w AS (
ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
PATTERN (A+) DEFINE A AS val > (random() * 0)::int);
The check is contain_volatile_functions() on wc->defineClause in
subquery_planner(), immediately after the clause is preprocessed. At
that point a DEFINE expression that matched a GROUP BY expression is a
GROUP RTE Var, so there is no volatile function to find.
flatten_group_exprs() expands those Vars back into the grouping
expressions further down, after the check has run.
I think that is the convention rather than a gap, on two counts. The
comment on the check states the first itself: volatility is examined
in the planner and not while parsing, and "a subquery the planner
discards before reaching this point is therefore not checked, which is
the same rule that lets a volatile fold away". The planner does not
go back and re-ask about what it has rearranged.
The second is what actually runs. In the accepted query above the
DEFINE condition reaches the executor as
OPEXPR(>) { Var(OUTER_VAR, 2), Var(OUTER_VAR, 3) }
That is val, and the grouped column. No volatile function is called
during matching: random() was evaluated once per row by the grouping,
and DEFINE reads the result the way it reads any other column. If the
grouping expression is genuinely random, DEFINE sees a random value,
but it sees the one the group already has.
So I have left it alone. If either of you reads the restriction as
covering this case, say so and I will write the extra check after the
flatten_group_exprs() loop.
Best regards,
Henson
From 41419d1ef7cf694b48101100f7bf3b5c16097159 Mon Sep 17 00:00:00 2001
From: Henson Choi <[email protected]>
Date: Fri, 18 Sep 2026 14:13:52 +0900
Subject: [PATCH] Keep a row pattern navigation offset out of the window input
A navigation offset is a run-time constant the executor resolves once,
at the top of the scan, before any input row has been read. But
set_upper_references() handed the whole DEFINE expression to
fix_upper_expr(), which matches any subexpression against the subplan's
targetlist. An offset spelled the same as something already in the
window input -- a window ORDER BY key, or a GROUP BY expression -- was
replaced with a Var(OUTER_VAR) referencing it, and resolve_nav_offsets()
then read that Var from an outer slot that is not set yet, dereferencing
a null TupleTableSlot.
Only the navigated argument is read from the input, one row at a time.
Give RPRNavExpr its own case in fix_upper_expr_mutator(): recurse into
arg as before, and hand the two offsets to fix_scan_expr(), which is
what set_plan_refs() already does with the WindowAgg frame offsets for
the same reason.
A constant offset was never affected, since
search_indexed_tlist_for_non_var() declines to match a Const. The
tests therefore use an offset that survives constant folding, once
against a window ORDER BY key and once against a GROUP BY expression.
---
src/backend/optimizer/plan/setrefs.c | 22 ++++++++++
src/test/regress/expected/rpr_base.out | 61 ++++++++++++++++++++++++++
src/test/regress/sql/rpr_base.sql | 34 ++++++++++++++
3 files changed, 117 insertions(+)
diff --git a/src/backend/optimizer/plan/setrefs.c
b/src/backend/optimizer/plan/setrefs.c
index 9dc191ad68e..2ee3c8baac2 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -3403,6 +3403,28 @@ fix_upper_expr_mutator(Node *node,
fix_upper_expr_context *context)
/* XXX can we assert something about phnullingrels? */
return fix_upper_expr_mutator((Node *) phv->phexpr, context);
}
+ if (IsA(node, RPRNavExpr))
+ {
+ RPRNavExpr *nav = (RPRNavExpr *) node;
+ RPRNavExpr *newnav = makeNode(RPRNavExpr);
+
+ memcpy(newnav, nav, sizeof(RPRNavExpr));
+
+ /*
+ * The offsets are resolved once per scan, before the outer
slot is
+ * set, so they cannot reference it the way arg does. Same
treatment
+ * as the WindowAgg frame offsets.
+ */
+ newnav->arg = (Expr *)
+ fix_upper_expr_mutator((Node *) nav->arg, context);
+ newnav->offset_arg = (Expr *)
+ fix_scan_expr(context->root, (Node *) nav->offset_arg,
+ context->rtoffset,
context->num_exec);
+ newnav->compound_offset_arg = (Expr *)
+ fix_scan_expr(context->root, (Node *)
nav->compound_offset_arg,
+ context->rtoffset,
context->num_exec);
+ return (Node *) newnav;
+ }
/* Try matching more complex expressions too, if tlist has any */
if (context->subplan_itlist->has_non_vars)
{
diff --git a/src/test/regress/expected/rpr_base.out
b/src/test/regress/expected/rpr_base.out
index bda37b4fc5f..f0d6cf1b924 100644
--- a/src/test/regress/expected/rpr_base.out
+++ b/src/test/regress/expected/rpr_base.out
@@ -2268,6 +2268,67 @@ WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND
UNBOUNDED FOLLOWING
DROP FUNCTION rpr_nav_dflt(int, int);
DROP FUNCTION rpr_nav_named(int, int);
DROP TABLE rpr_nav_txt;
+-- A navigation offset is resolved once at the top of the scan, before any
+-- input row has been read, so it must not be matched to the window input the
+-- way the navigated argument is. These two spell the offset the same as a
+-- window ORDER BY key and as a GROUP BY expression, which is what makes the
+-- match available.
+CREATE TABLE rpr_navoff (id int, val int);
+INSERT INTO rpr_navoff VALUES (1, 10), (2, 20), (3, 15), (4, 30), (5, 5);
+SELECT id, val, count(*) OVER w AS cnt
+FROM rpr_navoff
+WINDOW w AS (ORDER BY (extract(hour from localtimestamp)::int * 0 + 1), id
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ PATTERN (A B+)
+ DEFINE B AS val > PREV(val, (extract(hour from
localtimestamp)::int * 0 + 1)));
+ id | val | cnt
+----+-----+-----
+ 1 | 10 | 2
+ 2 | 20 | 0
+ 3 | 15 | 2
+ 4 | 30 | 0
+ 5 | 5 | 0
+(5 rows)
+
+-- Control: an offset that matches nothing in the window input.
+SELECT id, val, count(*) OVER w AS cnt
+FROM rpr_navoff
+WINDOW w AS (ORDER BY (extract(hour from localtimestamp)::int * 0 + 1), id
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ PATTERN (A B+)
+ DEFINE B AS val > PREV(val, (extract(hour from
localtimestamp)::int * 0 + 2)));
+ id | val | cnt
+----+-----+-----
+ 1 | 10 | 0
+ 2 | 20 | 3
+ 3 | 15 | 0
+ 4 | 30 | 0
+ 5 | 5 | 0
+(5 rows)
+
+SELECT id, val, count(*) OVER w AS cnt
+FROM rpr_navoff
+GROUP BY GROUPING SETS ((id, val, ((random() * 0)::bigint + 1)),
+ (id, ((random() * 0)::bigint + 1)))
+WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ PATTERN (A B+)
+ DEFINE B AS val > PREV(val, (random() * 0)::bigint + 1))
+ORDER BY id, val;
+ id | val | cnt
+----+-----+-----
+ 1 | 10 | 0
+ 1 | | 0
+ 2 | 20 | 0
+ 2 | | 0
+ 3 | 15 | 0
+ 3 | | 0
+ 4 | 30 | 0
+ 4 | | 0
+ 5 | 5 | 0
+ 5 | | 0
+(10 rows)
+
+DROP TABLE rpr_navoff;
-- 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 60625f6e6eb..1bf97a846b4 100644
--- a/src/test/regress/sql/rpr_base.sql
+++ b/src/test/regress/sql/rpr_base.sql
@@ -1611,6 +1611,40 @@ DROP FUNCTION rpr_nav_dflt(int, int);
DROP FUNCTION rpr_nav_named(int, int);
DROP TABLE rpr_nav_txt;
+-- A navigation offset is resolved once at the top of the scan, before any
+-- input row has been read, so it must not be matched to the window input the
+-- way the navigated argument is. These two spell the offset the same as a
+-- window ORDER BY key and as a GROUP BY expression, which is what makes the
+-- match available.
+CREATE TABLE rpr_navoff (id int, val int);
+INSERT INTO rpr_navoff VALUES (1, 10), (2, 20), (3, 15), (4, 30), (5, 5);
+
+SELECT id, val, count(*) OVER w AS cnt
+FROM rpr_navoff
+WINDOW w AS (ORDER BY (extract(hour from localtimestamp)::int * 0 + 1), id
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ PATTERN (A B+)
+ DEFINE B AS val > PREV(val, (extract(hour from
localtimestamp)::int * 0 + 1)));
+
+-- Control: an offset that matches nothing in the window input.
+SELECT id, val, count(*) OVER w AS cnt
+FROM rpr_navoff
+WINDOW w AS (ORDER BY (extract(hour from localtimestamp)::int * 0 + 1), id
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ PATTERN (A B+)
+ DEFINE B AS val > PREV(val, (extract(hour from
localtimestamp)::int * 0 + 2)));
+
+SELECT id, val, count(*) OVER w AS cnt
+FROM rpr_navoff
+GROUP BY GROUPING SETS ((id, val, ((random() * 0)::bigint + 1)),
+ (id, ((random() * 0)::bigint + 1)))
+WINDOW w AS (ORDER BY id ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ PATTERN (A B+)
+ DEFINE B AS val > PREV(val, (random() * 0)::bigint + 1))
+ORDER BY id, val;
+
+DROP TABLE rpr_navoff;
+
-- PREV function - reference previous row in pattern
SELECT id, val, COUNT(*) OVER w as cnt
FROM rpr_nav
--
2.54.0 (Apple Git-157)
