Re: Row pattern recognition - Mailing list pgsql-hackers

Hi Chao,

>> On Nov 21, 2025, at 13:25, Chao Li <li.evan.chao@gmail.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”.
 

I believe these names are based on the SQL standard. All syntax
components do not necessary be suffixed by "clause". For example
in select.sgml,

[ WITH [ RECURSIVE ] <replaceable class="parameter">with_query</replaceable> [, ...] ]
SELECT [ ALL | DISTINCT [ ON ( <replaceable class="parameter">expression</replaceable> [, ...] ) ] ]
    [ { * | <replaceable class="parameter">expression</replaceable> [ [ AS ] <replaceable
class="parameter">output_name</replaceable>] } [, ...] ]
 
    [ FROM <replaceable class="parameter">from_item</replaceable> [, ...] ]
    [ WHERE <replaceable class="parameter">condition</replaceable> ]
    [ GROUP BY { ALL | [ ALL | DISTINCT ] <replaceable class="parameter">grouping_element</replaceable> [, ...] } ]
    [ HAVING <replaceable class="parameter">condition</replaceable> ]
    [ WINDOW <replaceable class="parameter">window_name</replaceable> AS ( <replaceable
class="parameter">window_definition</replaceable>) [, ...] ]
 

"window_definition" comes from the standard and does not suffixed by
"clause". If you look into the window clause definition in the
standard, you will see:

<window clause> ::=
WINDOW <window definition list>
<window definition list> ::=
<window definition> [ { <comma> <window definition> }... ]

So the usage of terms in our document matches the standard. Likewise,
we see the definition of row pattern common syntax in the stabdard:

<window clause> ::=
WINDOW <window definition list>
<window definition list> ::=
<window definition> [ { <comma> <window definition> }... ]
<window definition> ::=
<new window name> AS <window specification>
<new window name> ::=
<window name>
<window specification> ::=
<left paren> <window specification details> <right paren>
<window specification details> ::=
[ <existing window name> ]
[ <window partition clause> ]
[ <window order clause> ]
[ <window frame clause> ]
:
:
<window frame clause> ::=
[ <row pattern measures> ]
<window frame units> <window frame extent>
[ <window frame exclusion> ]
[ <row pattern common syntax> ]

So I think "row pattern common syntax" is perfectly appropriate
name. If we change it to “row_pattern_recognition_clause”, it will
just bring confusion.  In the standard “row pattern recognition
clause” is defined RPR in FROM clause.

SELECT ... FROM table MATCH RECOGNIZE (....);

Here MATCH RECOGNIZE (....) is the “row pattern recognition clause”.

> 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.

Agreed. Will fix.

> 17 - 0006 - select.sgml
> ```
> PATTERN <replaceable class="parameter">pattern_variable_name</replaceable>[+] [, ...]
> ```
> 
> PATTERN definition should be placed inside (), here you missed ()

Good catch. Will fix.

> 18 - 0006 - select.sgml
> The same code as 17, <replaceable class="parameter">pattern_variable_name</replaceable>[+], do you only put “+” here,
Ithink 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).
> ```

Currently only *, + and ? are supported and I don't think it's worth
to invent "pattern_element".  (Actually the standard defines much more
complex syntax for PATTERN. I think we can revisit this once the basic
support for quantifier *,+ and ? are brought in the core PostgreSQL
code.

> 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.

Agreed. Will fix.

> 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.

Yes, row_pattern_common_syntax belongs to frame_clause. Will fix.

> 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
commentapplies to multiple places.
 

You are right. Will fix.

> I will proceed 0007 tomorrow.

Thanks!

Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp