public inbox for [email protected]  
help / color / mirror / Atom feed
From: Henson Choi <[email protected]>
To: Tatsuo Ishii <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Cc: [email protected]
Subject: Re: Row pattern recognition
Date: Sun, 1 Mar 2026 20:02:38 +0900
Message-ID: <CAAAe_zBdtRuAfyEX6eqtprY8ejGXqo6ndrpy0LPEHSzGWDvpxw@mail.gmail.com> (raw)
In-Reply-To: <CAAAe_zDq7R8CaDfmh8C+H3_639Y5LtJD+2Z=1txDt=vaOr90rQ@mail.gmail.com>
References: <[email protected]>
	<[email protected]>
	<CAAAe_zDaaOH4kaNfeN3Gk9u8VyTpUm-UgFebKc1KKB8o+PSjPg@mail.gmail.com>
	<[email protected]>
	<CAAAe_zDq7R8CaDfmh8C+H3_639Y5LtJD+2Z=1txDt=vaOr90rQ@mail.gmail.com>

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)



Attachments:

  [text/plain] nocfbot-0011-define-cost.txt (1.9K, 3-nocfbot-0011-define-cost.txt)
  download | inline diff:
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)



  [text/plain] nocfbot-0012-qualified-refs.txt (13.7K, 4-nocfbot-0012-qualified-refs.txt)
  download | inline diff:
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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Row pattern recognition
  In-Reply-To: <CAAAe_zBdtRuAfyEX6eqtprY8ejGXqo6ndrpy0LPEHSzGWDvpxw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox