Re: Row pattern recognition

Hi Tatsuo-san,

I just reviewed 0006 docs changes and 0001. I plan to slowly review the patch, maybe one commit a day. 

> On Nov 18, 2025, at 10:33, Tatsuo Ishii <ishii@postgresql.org> wrote:
> 
> Attached are the v35 patches for Row pattern recognition.  In v34-0001
> gram.y patch, %type for RPR were misplaced. v35-0001 fixes this. Other
> patches are not changed.
> 
> Best regards,
> --
> Tatsuo Ishii
> SRA OSS K.K.
> English: http://www.sraoss.co.jp/index_en/
> Japanese:http://www.sraoss.co.jp
> <v35-0001-Row-pattern-recognition-patch-for-raw-parser.patch><v35-0002-Row-pattern-recognition-patch-parse-analysis.patch><v35-0003-Row-pattern-recognition-patch-rewriter.patch><v35-0004-Row-pattern-recognition-patch-planner.patch><v35-0005-Row-pattern-recognition-patch-executor.patch><v35-0006-Row-pattern-recognition-patch-docs.patch><v35-0007-Row-pattern-recognition-patch-tests.patch><v35-0008-Row-pattern-recognition-patch-typedefs.list.patch>

I got a few comments, maybe just questions:

1 - 0001 - kwlist.h
```
+PG_KEYWORD("define", DEFINE, RESERVED_KEYWORD, BARE_LABEL)
```

Why do we add “define” as a reserved keyword? From the SQL example you put in 0006:
```
<programlisting>
SELECT company, tdate, price,
 first_value(price) OVER w,
 max(price) OVER w,
 count(price) OVER w
FROM stock
 WINDOW w AS (
 PARTITION BY company
 ORDER BY tdate
 ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
 AFTER MATCH SKIP PAST LAST ROW
 INITIAL
 PATTERN (LOWPRICE UP+ DOWN+)
 DEFINE
  LOWPRICE AS price &lt;= 100,
  UP AS price &gt; PREV(price),
  DOWN AS price &lt; PREV(price)
);
</programlisting>
```

PARTITION is at the same level as DEFINE, but it’s not defined as a reserved keyword:
```
PG_KEYWORD("partition", PARTITION, UNRESERVED_KEYWORD, BARE_LABEL)
```

Even in this patch,”initial”,”past”, “pattern” and “seek” are defined as unreserved, why?  

So I just want to clarify.

2 - 0001 - gram.y
```
opt_row_pattern_initial_or_seek:
			INITIAL		{ $$ = true; }
			| SEEK
				{
					ereport(ERROR,
							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
							 errmsg("SEEK is not supported"),
							 errhint("Use INITIAL instead."),
							 parser_errposition(@1)));
				}
			| /*EMPTY*/		{ $$ = true; }
		;
```

As SEEK is specially listed here, I guess it might be supported in future. If that is true, would it be better to defer the semantic check to later parse phase, which would future work easier.

3 - 0001 - parsenodes.h
```
+/*
+ * RowPatternCommonSyntax - raw representation of row pattern common syntax
+ *
+ */
+typedef struct RPCommonSyntax
+{
+	NodeTag		type;
+	RPSkipTo	rpSkipTo;		/* Row Pattern AFTER MATCH SKIP type */
+	bool		initial;		/* true if <row pattern initial or seek> is
+								 * initial */
+	List	   *rpPatterns;		/* PATTERN variables (list of A_Expr) */
+	List	   *rpDefs;			/* row pattern definitions clause (list of
+								 * ResTarget) */
+} RPCommonSyntax;
+
 /*
  * WindowDef - raw representation of WINDOW and OVER clauses
  *
@@ -593,6 +618,7 @@ typedef struct WindowDef
 	char	   *refname;		/* referenced window name, if any */
 	List	   *partitionClause;	/* PARTITION BY expression list */
 	List	   *orderClause;	/* ORDER BY (list of SortBy) */
+	RPCommonSyntax *rpCommonSyntax; /* row pattern common syntax */
```

RP fields are directly defined in WindowClause, then why do we need a wrapper of RPCommonSyntax? Can we directly define RP fields in WindowRef as well?

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/