nocfbot-0006-Fix-DEFINE-expression-handling-RPR-window-planning.txt
text/plain
Filename: nocfbot-0006-Fix-DEFINE-expression-handling-RPR-window-planning.txt
Type: text/plain
Part: 5
Message:
Re: Row pattern recognition
From b0f0184ef082dbd0a4744d08f6b0b7d5366c2733 Mon Sep 17 00:00:00 2001
From: Henson Choi <assam258@gmail.com>
Date: Thu, 2 Apr 2026 11:55:02 +0900
Subject: [PATCH 06/40] Fix DEFINE expression handling in RPR window planning
transformDefineClause() added full DEFINE expressions including
RPRNavExpr (PREV/NEXT) nodes to the query targetlist. These
propagated to upper WindowAgg nodes that lack RPR navigation state,
causing a SIGSEGV when RPR and non-RPR windows coexist in the same
query.
Add only the Var nodes referenced by DEFINE expressions to the
targetlist, and protect those Vars from removal by
remove_unused_subquery_outputs() so they remain available in the
tuplestore slot for pattern matching evaluation.
Move the subquery wrapping tests from rpr.sql to rpr_integration.sql.
---
src/backend/optimizer/path/allpaths.c | 68 +++++++++++++++++
src/backend/parser/parse_rpr.c | 106 ++++++++++++++------------
2 files changed, 126 insertions(+), 48 deletions(-)
diff --git a/src/backend/optimizer/path/allpaths.c b/src/backend/optimizer/path/allpaths.c
index f42a2bae14a..470029e42e0 100644
--- a/src/backend/optimizer/path/allpaths.c
+++ b/src/backend/optimizer/path/allpaths.c
@@ -4750,6 +4750,74 @@ remove_unused_subquery_outputs(Query *subquery, RelOptInfo *rel,
if (contain_volatile_functions(texpr))
continue;
+ /*
+ * If any RPR (Row Pattern Recognition) window clause references this
+ * column in its DEFINE clause, don't remove it. The DEFINE
+ * expression needs these columns in the tuplestore slot for pattern
+ * matching evaluation, even if the outer query doesn't reference
+ * them.
+ */
+ if (IsA(texpr, Var))
+ {
+ Var *var = (Var *) texpr;
+ ListCell *wlc;
+ bool needed_by_define = false;
+
+ foreach(wlc, subquery->windowClause)
+ {
+ WindowClause *wc = lfirst_node(WindowClause, wlc);
+
+ if (wc->defineClause != NIL)
+ {
+ List *vars = pull_var_clause((Node *) wc->defineClause, 0);
+ ListCell *vlc;
+
+ foreach(vlc, vars)
+ {
+ Var *dvar = (Var *) lfirst(vlc);
+
+ if (dvar->varattno == var->varattno)
+ {
+ needed_by_define = true;
+ break;
+ }
+ }
+ list_free(vars);
+ if (needed_by_define)
+ break;
+ }
+ }
+ if (needed_by_define)
+ continue;
+ }
+
+ /*
+ * If it's a window function referencing a window clause with RPR,
+ * don't remove it. Even when the window function result is unused by
+ * the outer query, the RPR pattern matching (frame reduction via
+ * DEFINE/PATTERN) must still execute. Replacing this with NULL would
+ * leave no active window functions for the WindowClause, causing the
+ * planner to omit the WindowAgg node entirely.
+ */
+ if (IsA(texpr, WindowFunc))
+ {
+ WindowFunc *wfunc = (WindowFunc *) texpr;
+ ListCell *wlc;
+
+ foreach(wlc, subquery->windowClause)
+ {
+ WindowClause *wc = lfirst_node(WindowClause, wlc);
+
+ if (wc->winref == wfunc->winref &&
+ wc->defineClause != NIL)
+ {
+ break;
+ }
+ }
+ if (wlc != NULL)
+ continue;
+ }
+
/*
* OK, we don't need it. Replace the expression with a NULL constant.
* Preserve the exposed type of the expression, in case something
diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c
index 55283ab4bbe..db1309ca311 100644
--- a/src/backend/parser/parse_rpr.c
+++ b/src/backend/parser/parse_rpr.c
@@ -28,6 +28,7 @@
#include "miscadmin.h"
#include "nodes/makefuncs.h"
#include "nodes/nodeFuncs.h"
+#include "optimizer/optimizer.h"
#include "optimizer/rpr.h"
#include "parser/parse_clause.h"
#include "parser/parse_collate.h"
@@ -310,9 +311,7 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
restargets = NIL;
foreach(lc, windef->rpCommonSyntax->rpDefs)
{
- TargetEntry *te,
- *teDefine;
- int defineExprLocation;
+ TargetEntry *teDefine;
restarget = (ResTarget *) lfirst(lc);
name = restarget->name;
@@ -335,57 +334,68 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
restargets = lappend(restargets, restarget);
/*
- * Transform the DEFINE expression (restarget->val) and add it to the
- * targetlist as a TargetEntry if not already present, so the planner
- * can propagate the referenced columns to the outer plan's
- * targetlist.
+ * Transform the DEFINE expression. We must NOT add the whole
+ * expression to the query targetlist, because it may contain
+ * RPRNavExpr nodes (PREV/NEXT) that can only be evaluated inside the
+ * owning WindowAgg.
*
- * Note: findTargetlistEntrySQL99 transforms and clobbers
- * restarget->val.
+ * Instead, we transform the expression directly and only ensure that
+ * the individual Var nodes it references are present in the
+ * targetlist, so the planner can propagate the referenced columns.
*/
+ {
+ Node *expr;
+ List *vars;
+ ListCell *lc2;
- /*
- * Save the original expression location before transformation.
- * findTargetlistEntrySQL99 may return an existing TargetEntry whose
- * location points to where it was originally created (e.g., ORDER
- * BY), not the DEFINE clause. We need to preserve the DEFINE location
- * for accurate error reporting.
- */
- defineExprLocation = exprLocation(restarget->val);
+ expr = transformExpr(pstate, restarget->val,
+ EXPR_KIND_RPR_DEFINE);
+
+ /*
+ * Pull out Var nodes from the transformed expression and ensure
+ * each one is present in the targetlist. This is needed so the
+ * planner propagates the referenced columns through the plan
+ * tree, making them available to the WindowAgg's DEFINE
+ * evaluation.
+ */
+ vars = pull_var_clause(expr, 0);
+ foreach(lc2, vars)
+ {
+ Var *var = (Var *) lfirst(lc2);
+ bool found = false;
+ ListCell *tl;
- te = findTargetlistEntrySQL99(pstate, restarget->val,
- targetlist, EXPR_KIND_RPR_DEFINE);
+ foreach(tl, *targetlist)
+ {
+ TargetEntry *tle = (TargetEntry *) lfirst(tl);
- /* -------------------
- * Copy the TargetEntry for defineClause and always set the pattern
- * variable name. We use copyObject so the original targetlist entry
- * is not modified.
- *
- * Note: We must always set resname to the pattern variable name.
- * findTargetlistEntrySQL99 creates new TEs with resname = NULL
- * (resjunk entries), but returns existing TEs unchanged when the
- * expression already exists in targetlist.
- *
- * Example: "SELECT id, flag, ... WINDOW w AS (... DEFINE T AS flag)"
- *
- * 1. SELECT list processing creates: TE{resname="flag", expr=flag}
- * 2. DEFINE T AS flag: findTargetlistEntrySQL99 finds existing TE
- * 3. te->resname is "flag" (from SELECT), not NULL
- * 4. Without unconditionally setting resname, teDefine->resname
- * would remain "flag" instead of pattern variable name "T"
- * 5. buildRPRPattern builds defineVariableList from resname, so
- * it would contain ["flag"] instead of ["T"]
- * 6. Pattern variable "T" not found -> Assert failure crash
- */
- teDefine = copyObject(te);
- teDefine->resname = pstrdup(name);
+ if (IsA(tle->expr, Var) &&
+ ((Var *) tle->expr)->varno == var->varno &&
+ ((Var *) tle->expr)->varattno == var->varattno)
+ {
+ found = true;
+ break;
+ }
+ }
+ if (!found)
+ {
+ TargetEntry *newtle;
- /*
- * Update the expression location to point to the DEFINE clause. This
- * ensures error messages reference the correct source location.
- */
- if (defineExprLocation >= 0 && IsA(teDefine->expr, Var))
- ((Var *) teDefine->expr)->location = defineExprLocation;
+ newtle = makeTargetEntry((Expr *) copyObject(var),
+ list_length(*targetlist) + 1,
+ NULL,
+ true);
+ *targetlist = lappend(*targetlist, newtle);
+ }
+ }
+ list_free(vars);
+
+ /* Build the defineClause entry directly from the transformed expr */
+ teDefine = makeTargetEntry((Expr *) expr,
+ list_length(defineClause) + 1,
+ pstrdup(name),
+ true);
+ }
/* build transformed DEFINE clause (list of TargetEntry) */
defineClause = lappend(defineClause, teDefine);
--
2.50.1 (Apple Git-155)