From 86e97a060e4d4182abc026540f667fbca7b18d95 Mon Sep 17 00:00:00 2001 From: jian he Date: Fri, 12 Jun 2026 18:49:17 +0800 Subject: [PATCH v47 6/6] refactor transformDefineClause The allowed nav-expression combination is limited, therefore, has_column_ref is not necessary. Column reference checks can use contain_var_clause, and it's cheap. Also, I made the following error message change: -ERROR: row pattern navigation offset must be a run-time constant +ERROR: row pattern navigation offset expression must not contain column references --- src/backend/parser/parse_rpr.c | 102 +++++++----------------------- src/test/regress/expected/rpr.out | 6 +- 2 files changed, 26 insertions(+), 82 deletions(-) diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c index 59624c7a46..c4d5449ea6 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; @@ -427,7 +424,6 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, 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); } @@ -491,12 +487,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; @@ -511,33 +501,39 @@ define_walker(Node *node, void *context) 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); } - - if (ctx->phase == DEFINE_PHASE_NAV_OFFSET) + else { /* - * Navs inside offset_arg are unusual but not directly banned; the - * constant-offset rule will catch any Var or volatile they - * contain. + * PHASE_BODY: this is an outer nav at top level. Walk arg first + * to collect nesting / column-ref state, then validate and (for + * compound forms) flatten, then walk offset(s). */ - return expression_tree_walker(node, define_walker, ctx); - } - - /* - * PHASE_BODY: this is an outer nav at top level. Walk arg first 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))); 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); @@ -579,17 +575,6 @@ define_walker(Node *node, void *context) nav->offset_arg = inner->offset_arg; nav->compound_offset_arg = nav->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, @@ -610,48 +595,7 @@ 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; } } diff --git a/src/test/regress/expected/rpr.out b/src/test/regress/expected/rpr.out index a07104153f..e3fb41459a 100644 --- a/src/test/regress/expected/rpr.out +++ b/src/test/regress/expected/rpr.out @@ -1204,7 +1204,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 @@ -1216,7 +1216,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 @@ -1228,7 +1228,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 -- 2.34.1