Hi Tatsuo,

Attached are two more incremental patches (nocfbot-0011, nocfbot-0012)
on top of v43, continuing the nocfbot-0001..0010 series.

nocfbot-0011: Add RPR DEFINE expression cost to WindowAgg cost estimation

  Revised version of your cost estimation patch [1].  Fixes the
  `(char *)` cast and moves DEFINE cost outside the windowFuncs loop.

nocfbot-0012: Reject qualified column references in RPR DEFINE clause

  Revised version of your qualified column reference patch [2].
  Exposes `patternVarNames` via `ParseState.p_rpr_pattern_vars` to
  distinguish pattern variable qualifiers ("not supported") from
  FROM-clause range variable qualifiers ("not allowed").

  Variables appearing only in DEFINE (not in PATTERN) are also
  collected into `p_rpr_pattern_vars` so they get the "not supported"
  message, but the RPR_VARID_MAX count check still applies only to
  PATTERN variables.

[1]
https://www.postgresql.org/message-id/[email protected]
[2]
https://www.postgresql.org/message-id/[email protected]

Attachment:
  nocfbot-0011-define-cost.txt
  nocfbot-0012-qualified-refs.txt

Best regards,
Henson
From 613d2b9cc310f9e55c71ef16555b9c42401c46b9 Mon Sep 17 00:00:00 2001
From: Henson Choi <[email protected]>
Date: Sun, 1 Mar 2026 19:09:27 +0900
Subject: [PATCH] Add RPR DEFINE expression cost to WindowAgg cost estimation

---
 src/backend/optimizer/path/costsize.c | 29 +++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/src/backend/optimizer/path/costsize.c 
b/src/backend/optimizer/path/costsize.c
index 89ca4e08bf1..a10fb802d0e 100644
--- a/src/backend/optimizer/path/costsize.c
+++ b/src/backend/optimizer/path/costsize.c
@@ -103,6 +103,7 @@
 #include "optimizer/placeholder.h"
 #include "optimizer/plancat.h"
 #include "optimizer/restrictinfo.h"
+#include "optimizer/rpr.h"
 #include "parser/parsetree.h"
 #include "utils/lsyscache.h"
 #include "utils/selfuncs.h"
@@ -3227,7 +3228,35 @@ cost_windowagg(Path *path, PlannerInfo *root,
         * many rows the window function will fetch, it's hard to do better.  In
         * any case, it's a good estimate for all the built-in window functions,
         * so we'll just do this for now.
+        *
+        * Moreover, if row pattern recognition is used, we charge the DEFINE
+        * expressions once per tuple for each variable that appears in PATTERN.
         */
+       if (winclause->rpPattern)
+       {
+               List       *pattern_vars;
+               ListCell   *lc2;
+               QualCost        defcosts;
+
+               pattern_vars = collectPatternVariables(winclause->rpPattern);
+
+               foreach(lc2, pattern_vars)
+               {
+                       char       *ptname = strVal(lfirst(lc2));
+
+                       foreach_node(TargetEntry, def, winclause->defineClause)
+                       {
+                               if (!strcmp(ptname, def->resname))
+                               {
+                                       cost_qual_eval_node(&defcosts, (Node *) 
def->expr, root);
+                                       startup_cost += defcosts.startup;
+                                       total_cost += defcosts.per_tuple * 
input_tuples;
+                               }
+                       }
+               }
+               list_free_deep(pattern_vars);
+       }
+
        foreach(lc, windowFuncs)
        {
                WindowFunc *wfunc = lfirst_node(WindowFunc, lc);
-- 
2.50.1 (Apple Git-155)

From f508104267e0c399fa48373e80ac51c8f4996103 Mon Sep 17 00:00:00 2001
From: Henson Choi <[email protected]>
Date: Sun, 1 Mar 2026 19:24:53 +0900
Subject: [PATCH] Reject qualified column references in RPR DEFINE clause

---
 src/backend/parser/parse_expr.c        | 36 ++++++++++++++
 src/backend/parser/parse_rpr.c         | 69 ++++++++++++++++++++------
 src/include/parser/parse_node.h        |  1 +
 src/test/regress/expected/rpr_base.out | 60 +++++++++++++++++-----
 src/test/regress/sql/rpr_base.sql      | 52 ++++++++++++++-----
 5 files changed, 178 insertions(+), 40 deletions(-)

diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 219681d6e88..99b7e4763aa 100644
--- a/src/backend/parser/parse_expr.c
+++ b/src/backend/parser/parse_expr.c
@@ -613,6 +613,42 @@ transformColumnRef(ParseState *pstate, ColumnRef *cref)
                        return node;
        }
 
+       /*
+        * Qualified column references in DEFINE are not supported.  This covers
+        * both FROM-clause range variables (prohibited by §6.5) and pattern
+        * variable qualified names (e.g. UP.price), which are valid per §4.16
+        * but not yet implemented.
+        */
+       if (pstate->p_expr_kind == EXPR_KIND_RPR_DEFINE &&
+               list_length(cref->fields) != 1)
+       {
+               char       *qualifier = strVal(linitial(cref->fields));
+               ListCell   *lc;
+               bool            is_pattern_var = false;
+
+               foreach(lc, pstate->p_rpr_pattern_vars)
+               {
+                       if (strcmp(strVal(lfirst(lc)), qualifier) == 0)
+                       {
+                               is_pattern_var = true;
+                               break;
+                       }
+               }
+
+               if (is_pattern_var)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                                        errmsg("pattern variable qualified 
column reference \"%s\" is not supported in DEFINE clause",
+                                                       
NameListToString(cref->fields)),
+                                        parser_errposition(pstate, 
cref->location)));
+               else
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_SYNTAX_ERROR),
+                                        errmsg("range variable qualified 
column reference \"%s\" is not allowed in DEFINE clause",
+                                                       
NameListToString(cref->fields)),
+                                        parser_errposition(pstate, 
cref->location)));
+       }
+
        /*----------
         * The allowed syntaxes are:
         *
diff --git a/src/backend/parser/parse_rpr.c b/src/backend/parser/parse_rpr.c
index 027a3d2500a..dff91e439d2 100644
--- a/src/backend/parser/parse_rpr.c
+++ b/src/backend/parser/parse_rpr.c
@@ -37,7 +37,7 @@
 
 /* Forward declarations */
 static void validateRPRPatternVarCount(ParseState *pstate, RPRPatternNode 
*node,
-                                                                          List 
**varNames);
+                                                                          List 
*rpDefs, List **varNames);
 static List *transformDefineClause(ParseState *pstate, WindowClause *wc,
                                                                   WindowDef 
*windef, List **targetlist);
 
@@ -160,11 +160,15 @@ transformRPR(ParseState *pstate, WindowClause *wc, 
WindowDef *windef,
  * Recursively traverses the pattern tree, collecting unique variable names.
  * Throws an error if the number of unique variables exceeds RPR_VARID_MAX.
  *
+ * If rpDefs is non-NULL, DEFINE variable names are also collected into
+ * varNames so that transformColumnRef can distinguish pattern variable
+ * qualifiers from FROM-clause range variables.
+ *
  * varNames is both input and output: existing names are preserved, new ones 
added.
  */
 static void
 validateRPRPatternVarCount(ParseState *pstate, RPRPatternNode *node,
-                                                  List **varNames)
+                                                  List *rpDefs, List 
**varNames)
 {
        ListCell   *lc;
 
@@ -209,18 +213,48 @@ validateRPRPatternVarCount(ParseState *pstate, 
RPRPatternNode *node,
                        foreach(lc, node->children)
                        {
                                validateRPRPatternVarCount(pstate, 
(RPRPatternNode *) lfirst(lc),
-                                                                               
   varNames);
+                                                                               
   NULL, varNames);
                        }
                        break;
        }
+
+       /*
+        * After the top-level call, also collect DEFINE variable names that are
+        * not already in the list.  This is only done once at the outermost
+        * recursion level, detected by rpDefs being non-NULL (recursive calls
+        * pass NULL).
+        */
+       if (rpDefs)
+       {
+               foreach(lc, rpDefs)
+               {
+                       ResTarget  *rt = (ResTarget *) lfirst(lc);
+                       ListCell   *lc2;
+                       bool            found = false;
+
+                       foreach(lc2, *varNames)
+                       {
+                               if (strcmp(strVal(lfirst(lc2)), rt->name) == 0)
+                               {
+                                       found = true;
+                                       break;
+                               }
+                       }
+                       if (!found)
+                               *varNames = lappend(*varNames,
+                                                                       
makeString(pstrdup(rt->name)));
+               }
+       }
 }
 
 /*
  * transformDefineClause
  *             Process DEFINE clause and transform ResTarget into list of 
TargetEntry.
  *
- * For each DEFINE variable:
- *   1. Validates PATTERN variable count via validateRPRPatternVarCount
+ * First:
+ *   1. Validates PATTERN variable count and collects RPR variable names
+ *
+ * Then for each DEFINE variable:
  *   2. Checks for duplicate variable names in DEFINE clause
  *   3. Transforms expressions and adds to targetlist via 
findTargetlistEntrySQL99
  *   4. Creates defineClause entry with proper resname (pattern variable name)
@@ -230,9 +264,9 @@ validateRPRPatternVarCount(ParseState *pstate, 
RPRPatternNode *node,
  * Note: Variables not in DEFINE are evaluated as TRUE by the executor.
  * Variables in DEFINE but not in PATTERN are filtered out by the planner.
  *
- * XXX we only support column reference in row pattern definition search
- * condition, e.g. "price". <row pattern definition variable name>.<column
- * reference> is not supported, e.g. "A.price".
+ * XXX Pattern variable qualified column references in DEFINE (e.g.
+ * "A.price") are not yet supported.  Currently rejected by
+ * transformColumnRef in parse_expr.c via the p_rpr_pattern_vars check.
  */
 static List *
 transformDefineClause(ParseState *pstate, WindowClause *wc, WindowDef *windef,
@@ -253,9 +287,14 @@ transformDefineClause(ParseState *pstate, WindowClause 
*wc, WindowDef *windef,
         */
        Assert(windef->rpCommonSyntax->rpDefs != NULL);
 
-       /* Validate PATTERN variable count (max RPR_VARID_MAX) */
+       /*
+        * Validate PATTERN variable count and collect all RPR variable names
+        * (PATTERN + DEFINE) for use in transformColumnRef.
+        */
        validateRPRPatternVarCount(pstate, windef->rpCommonSyntax->rpPattern,
+                                                          
windef->rpCommonSyntax->rpDefs,
                                                           &patternVarNames);
+       pstate->p_rpr_pattern_vars = patternVarNames;
 
        /*
         * Check for duplicate row pattern definition variables.  The standard
@@ -290,13 +329,12 @@ transformDefineClause(ParseState *pstate, WindowClause 
*wc, WindowDef *windef,
                restargets = lappend(restargets, restarget);
 
                /*
-                * Add DEFINE expression (Restarget->val) to the targetlist as a
-                * TargetEntry if it does not exist yet. Planner will add the 
column
-                * ref var node to the outer plan's target list later on. This 
makes
-                * DEFINE expression could access the outer tuple while 
evaluating
-                * PATTERN.
+                * 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.
                 *
-                * Note: findTargetlistEntrySQL99 does Expr transformation and 
clobber
+                * Note: findTargetlistEntrySQL99 transforms and clobbers
                 * restarget->val.
                 */
 
@@ -347,6 +385,7 @@ transformDefineClause(ParseState *pstate, WindowClause *wc, 
WindowDef *windef,
                defineClause = lappend(defineClause, teDefine);
        }
        list_free(restargets);
+       pstate->p_rpr_pattern_vars = NIL;
 
        /*
         * Make sure that the row pattern definition search condition is a 
boolean
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 4dc7e5738ae..f55d4c14b0a 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -208,6 +208,7 @@ struct ParseState
        ParseNamespaceItem *p_grouping_nsitem;  /* NSItem for grouping, or NULL 
*/
        List       *p_windowdefs;       /* raw representations of window 
clauses */
        ParseExprKind p_expr_kind;      /* what kind of expression we're 
parsing */
+       List       *p_rpr_pattern_vars; /* RPR variable names for DEFINE clause 
*/
        int                     p_next_resno;   /* next targetlist resno to 
assign */
        List       *p_multiassign_exprs;        /* junk tlist entries for 
multiassign */
        List       *p_locking_clause;   /* raw FOR UPDATE/FOR SHARE info */
diff --git a/src/test/regress/expected/rpr_base.out 
b/src/test/regress/expected/rpr_base.out
index 43eea32130f..ab878443379 100644
--- a/src/test/regress/expected/rpr_base.out
+++ b/src/test/regress/expected/rpr_base.out
@@ -2682,9 +2682,7 @@ LINE 6:     DEFINE A AS val > 0
             ^
 -- Expected: Syntax error
 -- Qualified column references (NOT SUPPORTED)
--- Pattern variables in DEFINE clause cannot use qualified references (A.price)
--- This gives a confusing error about missing FROM-clause entry
--- Qualified reference in DEFINE clause
+-- Pattern variable qualified name: not supported (valid per §4.16, not yet 
implemented)
 SELECT COUNT(*) OVER w
 FROM rpr_err
 WINDOW w AS (
@@ -2693,10 +2691,45 @@ WINDOW w AS (
     PATTERN (A+)
     DEFINE A AS A.val > 0
 );
-ERROR:  missing FROM-clause entry for table "a"
+ERROR:  pattern variable qualified column reference "a.val" is not supported 
in DEFINE clause
 LINE 7:     DEFINE A AS A.val > 0
                         ^
--- Expected: ERROR: missing FROM-clause entry for table "a"
+-- PATTERN-only variable qualified name: not supported even without DEFINE 
entry
+SELECT COUNT(*) OVER w
+FROM rpr_err
+WINDOW w AS (
+    ORDER BY id
+    ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+    PATTERN (A+ B+)
+    DEFINE A AS B.val > 0
+);
+ERROR:  pattern variable qualified column reference "b.val" is not supported 
in DEFINE clause
+LINE 7:     DEFINE A AS B.val > 0
+                        ^
+-- DEFINE-only variable qualified name: still a pattern variable, not a range 
variable
+SELECT COUNT(*) OVER w
+FROM rpr_err
+WINDOW w AS (
+    ORDER BY id
+    ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+    PATTERN (A+)
+    DEFINE A AS val > 0, B AS B.val > 0
+);
+ERROR:  pattern variable qualified column reference "b.val" is not supported 
in DEFINE clause
+LINE 7:     DEFINE A AS val > 0, B AS B.val > 0
+                                      ^
+-- FROM-clause range variable qualified name: not allowed (prohibited by §6.5)
+SELECT COUNT(*) OVER w
+FROM rpr_err
+WINDOW w AS (
+    ORDER BY id
+    ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+    PATTERN (A+)
+    DEFINE A AS rpr_err.val > 0
+);
+ERROR:  range variable qualified column reference "rpr_err.val" is not allowed 
in DEFINE clause
+LINE 7:     DEFINE A AS rpr_err.val > 0
+                        ^
 -- Semantic errors
 -- Undefined column in DEFINE
 SELECT COUNT(*) OVER w
@@ -4674,8 +4707,8 @@ WINDOW w AS (
     ORDER BY t1.id
     ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
     PATTERN (A+ B)
-    DEFINE A AS t1.val1 > 20,
-           B AS t2.val2 > 200
+    DEFINE A AS val1 > 20,
+           B AS val2 > 200
 )
 ORDER BY t1.id;
  id | val1 | val2 | cnt 
@@ -4709,17 +4742,18 @@ ORDER BY t1.id, t2.id;
 (4 rows)
 
 -- Self-Join with RPR
-SELECT a.id, a.val1, b.val1 as val1_next,
+SELECT id, val1, val1_next,
        COUNT(*) OVER w as cnt
-FROM rpr_join1 a
-INNER JOIN rpr_join1 b ON a.id + 1 = b.id
+FROM (SELECT a.id, a.val1, b.val1 as val1_next
+      FROM rpr_join1 a
+      INNER JOIN rpr_join1 b ON a.id + 1 = b.id) sub
 WINDOW w AS (
-    ORDER BY a.id
+    ORDER BY id
     ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
     PATTERN (X+)
-    DEFINE X AS a.val1 < b.val1
+    DEFINE X AS val1 < val1_next
 )
-ORDER BY a.id;
+ORDER BY id;
  id | val1 | val1_next | cnt 
 ----+------+-----------+-----
   1 |   10 |        20 |   4
diff --git a/src/test/regress/sql/rpr_base.sql 
b/src/test/regress/sql/rpr_base.sql
index 992c081205a..97b62a73a0e 100644
--- a/src/test/regress/sql/rpr_base.sql
+++ b/src/test/regress/sql/rpr_base.sql
@@ -1891,10 +1891,8 @@ WINDOW w AS (
 -- Expected: Syntax error
 
 -- Qualified column references (NOT SUPPORTED)
--- Pattern variables in DEFINE clause cannot use qualified references (A.price)
--- This gives a confusing error about missing FROM-clause entry
 
--- Qualified reference in DEFINE clause
+-- Pattern variable qualified name: not supported (valid per §4.16, not yet 
implemented)
 SELECT COUNT(*) OVER w
 FROM rpr_err
 WINDOW w AS (
@@ -1903,7 +1901,36 @@ WINDOW w AS (
     PATTERN (A+)
     DEFINE A AS A.val > 0
 );
--- Expected: ERROR: missing FROM-clause entry for table "a"
+
+-- PATTERN-only variable qualified name: not supported even without DEFINE 
entry
+SELECT COUNT(*) OVER w
+FROM rpr_err
+WINDOW w AS (
+    ORDER BY id
+    ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+    PATTERN (A+ B+)
+    DEFINE A AS B.val > 0
+);
+
+-- DEFINE-only variable qualified name: still a pattern variable, not a range 
variable
+SELECT COUNT(*) OVER w
+FROM rpr_err
+WINDOW w AS (
+    ORDER BY id
+    ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+    PATTERN (A+)
+    DEFINE A AS val > 0, B AS B.val > 0
+);
+
+-- FROM-clause range variable qualified name: not allowed (prohibited by §6.5)
+SELECT COUNT(*) OVER w
+FROM rpr_err
+WINDOW w AS (
+    ORDER BY id
+    ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+    PATTERN (A+)
+    DEFINE A AS rpr_err.val > 0
+);
 
 -- Semantic errors
 
@@ -2983,8 +3010,8 @@ WINDOW w AS (
     ORDER BY t1.id
     ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
     PATTERN (A+ B)
-    DEFINE A AS t1.val1 > 20,
-           B AS t2.val2 > 200
+    DEFINE A AS val1 > 20,
+           B AS val2 > 200
 )
 ORDER BY t1.id;
 
@@ -3005,17 +3032,18 @@ ORDER BY t1.id, t2.id;
 
 -- Self-Join with RPR
 
-SELECT a.id, a.val1, b.val1 as val1_next,
+SELECT id, val1, val1_next,
        COUNT(*) OVER w as cnt
-FROM rpr_join1 a
-INNER JOIN rpr_join1 b ON a.id + 1 = b.id
+FROM (SELECT a.id, a.val1, b.val1 as val1_next
+      FROM rpr_join1 a
+      INNER JOIN rpr_join1 b ON a.id + 1 = b.id) sub
 WINDOW w AS (
-    ORDER BY a.id
+    ORDER BY id
     ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
     PATTERN (X+)
-    DEFINE X AS a.val1 < b.val1
+    DEFINE X AS val1 < val1_next
 )
-ORDER BY a.id;
+ORDER BY id;
 
 DROP TABLE rpr_join1, rpr_join2;
 
-- 
2.50.1 (Apple Git-155)

Reply via email to