v50-0002-refactor-define_walker.nocfbot
application/octet-stream
Filename: v50-0002-refactor-define_walker.nocfbot
Type: application/octet-stream
Part: 2
Message:
Re: Row pattern recognition
From a11ecb59362eca56f8960d44337df5750e975ee4 Mon Sep 17 00:00:00 2001
From: jian he <jian.universality@gmail.com>
Date: Thu, 2 Jul 2026 11:49:05 +0800
Subject: [PATCH v50 2/3] refactor define_walker
Based on https://github.com/assam258-5892/postgres/commits/RPR
The allowed nav-expression combination is limited, therefore, has_column_ref is
not necessary. We can use contain_var_clause to do column reference checks, and
it's cheap.
Remove DefinePhase.DEFINE_PHASE_NAV_OFFSET, so define_walker is less recursive,
more readabale.
errdetail("A navigation offset must be a run-time constant.")
ERROR: row pattern navigation offset must be a run-time constant
The error message seems wrong, because the offset can be a plpgsql STABLE
function.
Discussion: https://postgr.es/m/CACJufxG57=ddtbN=5RZCzhxWDYXvocKmB7NtZy+DoqZuhxb_DA@mail.gmail.com
---
src/backend/parser/parse_rpr.c | 133 +++++++++----------------
src/test/regress/expected/rpr.out | 6 +-
src/test/regress/expected/rpr_base.out | 12 +--
3 files changed, 53 insertions(+), 98 deletions(-)
diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c
index 8cf6d49e90..37b3475e18 100644
--- a/src/backend/parser/parse_rpr.c
+++ b/src/backend/parser/parse_rpr.c
@@ -38,8 +38,6 @@ typedef enum
{
DEFINE_PHASE_BODY, /* top-level DEFINE expression */
DEFINE_PHASE_NAV_ARG, /* inside an outer nav's arg subtree */
- DEFINE_PHASE_NAV_OFFSET, /* inside an outer nav's offset_arg /
- * compound_offset_arg */
} DefinePhase;
typedef struct
@@ -47,7 +45,6 @@ typedef struct
ParseState *pstate;
DefinePhase phase;
int nav_count; /* RPRNavExpr nodes seen in current nav.arg */
- bool has_column_ref; /* Var seen in current nav scope */
RPRNavKind inner_kind; /* kind of first nested nav in current arg */
} DefineWalkCtx;
@@ -57,6 +54,7 @@ static void validateRPRPatternVarCount(ParseState *pstate, RPRPatternNode *node,
static List *transformDefineClause(ParseState *pstate, WindowDef *windef,
List **targetlist);
static bool define_walker(Node *node, void *context);
+static bool contain_rprnavexpr(Node *node);
/*
* transformRPR
@@ -404,8 +402,7 @@ transformDefineClause(ParseState *pstate, WindowDef *windef,
/*
* Validate DEFINE expressions: nested PREV/NEXT, column references,
- * compound flatten, volatile callees -- all in a single walk per
- * variable.
+ * compound flatten -- all in a single walk per variable.
*/
foreach_ptr(TargetEntry, te, defineClause)
{
@@ -414,7 +411,6 @@ transformDefineClause(ParseState *pstate, WindowDef *windef,
ctx.pstate = pstate;
ctx.phase = DEFINE_PHASE_BODY;
ctx.nav_count = 0;
- ctx.has_column_ref = false;
ctx.inner_kind = 0;
(void) define_walker((Node *) te->expr, &ctx);
}
@@ -447,9 +443,8 @@ transformDefineClause(ParseState *pstate, WindowDef *windef,
* body (top-level), inside a nav.arg, or inside a nav.offset_arg /
* compound_offset_arg. When entering an outer nav (PHASE_BODY), it
* walks nav.arg in PHASE_NAV_ARG to collect nesting/column-ref state,
- * applies compound flatten or raises a nesting error, then walks the
- * (post-flatten) offset(s) in PHASE_NAV_OFFSET to enforce the
- * constant-offset and no-nested-nav rules. No subtree is walked twice.
+ * applies compound flatten or raises a nesting error.
+ * No subtree is walked twice.
*/
/*
@@ -463,8 +458,8 @@ transformDefineClause(ParseState *pstate, WindowDef *windef,
* NEXT_LAST)
* - any other nesting is rejected (FIRST(PREV()),
* PREV(PREV()), FIRST(FIRST()), three-or-more deep)
- * [2] for each nav offset (PHASE_NAV_OFFSET):
- * - must be a run-time constant (no column references)
+ * [2] for each nav offset
+ * - must not contain column references
* - must not contain a row pattern navigation operation
*
* Var sightings feed the column-ref rule for the enclosing nav scope;
@@ -480,12 +475,6 @@ define_walker(Node *node, void *context)
if (node == NULL)
return false;
- /* Var sighting feeds the column-ref rule for the enclosing nav scope. */
- if (IsA(node, Var) &&
- (ctx->phase == DEFINE_PHASE_NAV_ARG ||
- ctx->phase == DEFINE_PHASE_NAV_OFFSET))
- ctx->has_column_ref = true;
-
if (IsA(node, RPRNavExpr))
{
RPRNavExpr *nav = (RPRNavExpr *) node;
@@ -495,25 +484,20 @@ define_walker(Node *node, void *context)
/*
* Nested nav inside an outer nav.arg: record for the outer's
* compound / nesting decision, then keep recursing so deeper Vars
- * and volatile callees are still observed.
+ * are still observed.
*/
if (ctx->nav_count == 0)
ctx->inner_kind = nav->kind;
ctx->nav_count++;
+
+ if (contain_var_clause((Node *) nav->offset_arg))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("row pattern navigation offset expression must not contain column references"),
+ parser_errposition(ctx->pstate, exprLocation((Node *) nav->offset_arg)));
+
return expression_tree_walker(node, define_walker, ctx);
}
- else if (ctx->phase == DEFINE_PHASE_NAV_OFFSET)
- {
- /*
- * A navigation offset must be a run-time constant, so it cannot
- * contain a navigation operation.
- */
- ereport(ERROR,
- errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("row pattern navigation offset cannot contain a row pattern navigation operation"),
- errdetail("A navigation offset must be a run-time constant."),
- parser_errposition(ctx->pstate, nav->location));
- }
else
{
/*
@@ -521,14 +505,29 @@ define_walker(Node *node, void *context)
* to collect nesting / column-ref state, then validate and (for
* compound forms) flatten, then walk offset(s).
*/
- DefineWalkCtx saved = *ctx;
bool outer_phys = (nav->kind == RPR_NAV_PREV ||
nav->kind == RPR_NAV_NEXT);
- bool flattened = false;
+
+ if (!contain_var_clause((Node *) nav->arg))
+ ereport(ERROR,
+ errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("argument of row pattern navigation operation must include at least one column reference"),
+ parser_errposition(ctx->pstate, nav->location));
+
+ if (contain_var_clause((Node *) nav->offset_arg))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("row pattern navigation offset expression must not contain column references"),
+ parser_errposition(ctx->pstate, exprLocation((Node *) nav->offset_arg)));
+
+ if (contain_rprnavexpr((Node *) nav->offset_arg))
+ ereport(ERROR,
+ errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("row pattern navigation offset cannot contain a row pattern navigation operation"),
+ parser_errposition(ctx->pstate, exprLocation((Node *) nav->offset_arg)));
ctx->phase = DEFINE_PHASE_NAV_ARG;
ctx->nav_count = 0;
- ctx->has_column_ref = false;
ctx->inner_kind = 0;
(void) define_walker((Node *) nav->arg, ctx);
@@ -570,17 +569,6 @@ define_walker(Node *node, void *context)
nav->compound_offset_arg = nav->offset_arg;
nav->offset_arg = inner->offset_arg;
nav->arg = inner->arg;
- flattened = true;
-
- /*
- * The flattened argument must include a column reference,
- * just like the simple-nav case below.
- */
- if (!ctx->has_column_ref)
- ereport(ERROR,
- errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("argument of row pattern navigation operation must include at least one column reference"),
- parser_errposition(ctx->pstate, nav->location));
}
else if (!outer_phys && inner_phys)
ereport(ERROR,
@@ -601,51 +589,26 @@ define_walker(Node *node, void *context)
errhint("Only PREV(FIRST()), PREV(LAST()), NEXT(FIRST()), and NEXT(LAST()) compound forms are allowed."),
parser_errposition(ctx->pstate, nav->location));
}
- else if (!ctx->has_column_ref)
- {
- ereport(ERROR,
- errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("argument of row pattern navigation operation must include at least one column reference"),
- parser_errposition(ctx->pstate, nav->location));
- }
-
- /*
- * Walk offset arg(s) in PHASE_NAV_OFFSET to enforce the
- * constant-offset rule. For compound forms, both the inner
- * (post-flatten nav->offset_arg) and outer (compound_offset_arg)
- * offsets must be constants; the inner's column-ref status was
- * not separately tracked during the PHASE_NAV_ARG walk (which
- * only checks that nav.arg as a whole has at least one Var), so
- * it is re-walked here to catch column references the inner
- * offset would have leaked.
- */
- ctx->phase = DEFINE_PHASE_NAV_OFFSET;
-
- if (nav->offset_arg != NULL)
- {
- ctx->has_column_ref = false;
- (void) define_walker((Node *) nav->offset_arg, ctx);
- if (ctx->has_column_ref)
- ereport(ERROR,
- errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("row pattern navigation offset must be a run-time constant"),
- parser_errposition(ctx->pstate, exprLocation((Node *) nav->offset_arg)));
- }
- if (flattened && nav->compound_offset_arg != NULL)
- {
- ctx->has_column_ref = false;
- (void) define_walker((Node *) nav->compound_offset_arg, ctx);
- if (ctx->has_column_ref)
- ereport(ERROR,
- errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("row pattern navigation offset must be a run-time constant"),
- parser_errposition(ctx->pstate, exprLocation((Node *) nav->compound_offset_arg)));
- }
- *ctx = saved;
return false;
}
}
return expression_tree_walker(node, define_walker, ctx);
}
+
+static bool
+contain_rprnavexpr_walker(Node *node, void *context)
+{
+ if (node == NULL)
+ return false;
+ if (IsA(node, RPRNavExpr))
+ return true;
+ return expression_tree_walker(node, contain_rprnavexpr_walker, context);
+}
+
+static bool
+contain_rprnavexpr(Node *node)
+{
+ return contain_rprnavexpr_walker(node, NULL);
+}
diff --git a/src/test/regress/expected/rpr.out b/src/test/regress/expected/rpr.out
index 241937b44a..e36cad830f 100644
--- a/src/test/regress/expected/rpr.out
+++ b/src/test/regress/expected/rpr.out
@@ -1181,7 +1181,7 @@ WINDOW w AS (
PATTERN (A)
DEFINE A AS PREV(price, price) > 0
);
-ERROR: row pattern navigation offset must be a run-time constant
+ERROR: row pattern navigation offset expression must not contain column references
LINE 7: DEFINE A AS PREV(price, price) > 0
^
-- Non-constant offset: column reference in compound inner offset
@@ -1193,7 +1193,7 @@ WINDOW w AS (
PATTERN (A)
DEFINE A AS PREV(LAST(price, price), 2) > 0
);
-ERROR: row pattern navigation offset must be a run-time constant
+ERROR: row pattern navigation offset expression must not contain column references
LINE 7: DEFINE A AS PREV(LAST(price, price), 2) > 0
^
-- Non-constant offset: column reference in compound outer offset
@@ -1205,7 +1205,7 @@ WINDOW w AS (
PATTERN (A)
DEFINE A AS PREV(LAST(price, 1), price) > 0
);
-ERROR: row pattern navigation offset must be a run-time constant
+ERROR: row pattern navigation offset expression must not contain column references
LINE 7: DEFINE A AS PREV(LAST(price, 1), price) > 0
^
-- Non-constant offset: volatile function as offset
diff --git a/src/test/regress/expected/rpr_base.out b/src/test/regress/expected/rpr_base.out
index 1ce63a40ca..37cddcc9d1 100644
--- a/src/test/regress/expected/rpr_base.out
+++ b/src/test/regress/expected/rpr_base.out
@@ -2276,7 +2276,6 @@ SELECT id, val
ERROR: row pattern navigation offset cannot contain a row pattern navigation operation
LINE 6: DEFINE A AS PREV(val, FIRST(1)) > 0)
^
-DETAIL: A navigation offset must be a run-time constant.
DROP SCHEMA rpr_navns CASCADE;
RESET search_path;
-- ============================================================
@@ -4141,7 +4140,6 @@ WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
ERROR: row pattern navigation offset cannot contain a row pattern navigation operation
LINE 4: PATTERN (A+) DEFINE A AS PREV(v, FIRST(1)) > 0);
^
-DETAIL: A navigation offset must be a run-time constant.
SELECT count(*) OVER w
FROM generate_series(1,10) s(v)
WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
@@ -4149,7 +4147,6 @@ WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
ERROR: row pattern navigation offset cannot contain a row pattern navigation operation
LINE 4: PATTERN (A+) DEFINE A AS PREV(v, FIRST(1) + 1) > 0);
^
-DETAIL: A navigation offset must be a run-time constant.
SELECT count(*) OVER w
FROM generate_series(1,10) s(v)
WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
@@ -4157,7 +4154,6 @@ WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
ERROR: row pattern navigation offset cannot contain a row pattern navigation operation
LINE 4: PATTERN (A+) DEFINE A AS PREV(v, NEXT(1, 0)) > 0);
^
-DETAIL: A navigation offset must be a run-time constant.
SELECT count(*) OVER w
FROM generate_series(1,10) s(v)
WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
@@ -4165,23 +4161,20 @@ WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
ERROR: row pattern navigation offset cannot contain a row pattern navigation operation
LINE 4: PATTERN (A+) DEFINE A AS PREV(FIRST(v), LAST(1)) > 0);
^
-DETAIL: A navigation offset must be a run-time constant.
SELECT count(*) OVER w
FROM generate_series(1,10) s(v)
WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
PATTERN (A+) DEFINE A AS PREV(v, FIRST(v)) > 0);
-ERROR: row pattern navigation offset cannot contain a row pattern navigation operation
+ERROR: row pattern navigation offset expression must not contain column references
LINE 4: PATTERN (A+) DEFINE A AS PREV(v, FIRST(v)) > 0);
^
-DETAIL: A navigation offset must be a run-time constant.
SELECT count(*) OVER w
FROM generate_series(1,10) s(v)
WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
PATTERN (A+) DEFINE A AS NEXT(v, PREV(v, 1)) > 0);
-ERROR: row pattern navigation offset cannot contain a row pattern navigation operation
+ERROR: row pattern navigation offset expression must not contain column references
LINE 4: PATTERN (A+) DEFINE A AS NEXT(v, PREV(v, 1)) > 0);
^
-DETAIL: A navigation offset must be a run-time constant.
SELECT count(*) OVER w
FROM generate_series(1,10) s(v)
WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
@@ -4197,7 +4190,6 @@ WINDOW w AS (ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
ERROR: row pattern navigation offset cannot contain a row pattern navigation operation
LINE 4: PATTERN (A+) DEFINE A AS PREV(v, FIRST(1::bigint)) > 0);
^
-DETAIL: A navigation offset must be a run-time constant.
-- An unknown literal argument resolves to text; it must still reference a column
SELECT count(*) OVER w
FROM generate_series(1,5) s(v)
--
2.34.1