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(at)postgresql(dot)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/