Re: Row pattern recognition

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 &lt;= 100,
UP AS price &gt; PREV(price),
DOWN AS price &lt; 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/

In response to

Responses

Browse pgsql-hackers by date

  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