Thread
-
Re: Row pattern recognition
Tatsuo Ishii <ishii@postgresql.org> — 2025-11-24T06:47:03Z
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, 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). > ``` 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 comment applies 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