| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Vik Fearing <vik(at)postgresfriends(dot)org>, Tatsuo Ishii <ishii(at)postgresql(dot)org> |
| Cc: | david(dot)g(dot)johnston(at)gmail(dot)com, jacob(dot)champion(at)enterprisedb(dot)com, er(at)xs4all(dot)nl, peter(at)eisentraut(dot)org, pgsql-hackers(at)postgresql(dot)org |
| Subject: | Re: Row pattern recognition |
| Date: | 2025-11-24 00:59:57 |
| Message-ID: | 3AEE9E8E-FCE6-42A0-99E9-A5AB732030E9@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Nov 21, 2025, at 13:25, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> wrote:
>
>
> Okay, I’d stop here and continue to review 0006 next week.
>
I just finished reviewing 0006, and see some problems:
15 - 0006 - select.sgml
```
+[ <replaceable class="parameter">row_pattern_common_syntax</replaceable> ]
```
row_pattern_common_syntax doesn’t look like a good name. I searched over the docs, and couldn't find a name suffixed by “_syntax”. I think we can just name it as “row_pattern_recognition_clause” or a shorter name just “row_pattern”.
16 - 0006 - select.sgml
```
+ <synopsis>
+ [ AFTER MATCH SKIP PAST LAST ROW | AFTER MATCH SKIP TO NEXT ROW ]
``
I think the two values are mutually exclusive, so curly braces should added surround them:
[ { AFTER MATCH SKIP PAST LAST ROW | AFTER MATCH SKIP TO NEXT ROW } ]
[] means optional, and {} means choose one from all alternatives.
17 - 0006 - select.sgml
```
PATTERN <replaceable class="parameter">pattern_variable_name</replaceable>[+] [, ...]
```
PATTERN definition should be placed inside (), here you missed ()
18 - 0006 - select.sgml
The same code as 17, <replaceable class="parameter">pattern_variable_name</replaceable>[+], do you only put “+” here, I think SQL allows a lot of pattern quantifiers. From 0001, gram.y, looks like +, * and ? Are supported in this patch. So, maybe we can do:
```
PATTERN ( pattern_element, [pattern_element …] )
and pattern_element = variable name optionally followed by quantifier (reference to somewhere introducing pattern quantifier).
```
19 - 0006 - select.sgml
I don’t see INITIAL and SEEK are described. Even if SEEK is not supported currently, it’s worthy mentioning that.
20 - 0006 - select.sgml
```
+ after a match found. With <literal>AFTER MATCH SKIP PAST LAST
+ ROW</literal> (the default) next row position is next to the last row of
```
21 - 0006 - select.sgml
```
[ <replaceable class="parameter">frame_clause</replaceable> ]
+[ <replaceable class="parameter">row_pattern_common_syntax</replaceable> ]
```
Looks like row_pattern_common_syntax belongs to frame_clause, and you have lately added row_pattern_common_syntax to frame_clause:
```
<synopsis>
-{ RANGE | ROWS | GROUPS } <replaceable>frame_start</replaceable> [ <replaceable>frame_exclusion</replaceable> ]
-{ RANGE | ROWS | GROUPS } BETWEEN <replaceable>frame_start</replaceable> AND <replaceable>frame_end</replaceable> [ <replaceable>frame_exclusion</replaceable> ]
+{ RANGE | ROWS | GROUPS } <replaceable>frame_start</replaceable> [ <replaceable>frame_exclusion</replaceable> ] [row_pattern_common_syntax]
+{ RANGE | ROWS | GROUPS } BETWEEN <replaceable>frame_start</replaceable> AND <replaceable>frame_end</replaceable> [ <replaceable>frame_exclusion</replaceable> ] [row_pattern_common_syntax]
</synopsis>
```
So I guess row_pattern_common_syntax after frame_clause should not be added.
22 - 0006 - advance.sgml
```
<programlisting>
DEFINE
LOWPRICE AS price <= 100,
UP AS price > PREV(price),
DOWN AS price < PREV(price)
</programlisting>
Note that <function>PREV</function> returns the price column in the
```
In the “Note” line, “price” refers to a column from above example, so I think it should be tagged by <literal>. This comment applies to multiple places.
I will proceed 0007 tomorrow.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Junwang Zhao | 2025-11-24 01:57:03 | Re: Adjust comments for `IndexOptInfo` to accurately reflect indexcollations's length |
| Previous Message | Michael Paquier | 2025-11-24 00:10:29 | Re: [Proposal] Adding callback support for custom statistics kinds |