> I just finished reviewing 0007 and 0008. This patch set really demonstrates
> the full lifecycle of adding a SQL feature, from changing the syntax in
> gram.y all the way down to the executor, including tests and docs. I learned
> a lot from it. Thanks!
You are welcome!
> 23 - 0007
>
> As you mentioned earlier, pattern regular expression support *, + and ?, but
> I don’t see ? is tested.
Good catch. I will add the test case. In the mean time I found a bug
with gram.y part which handles '?' quantifier. gram.y can be
successfully compiled but there's no way to put '?' quantifier in a
SQL statement. We cannot write directly "ColId '?'" like other
quantifier '*' and '+' in the grammer because '?' is not a "self"
token. So we let '?' matches Op first then check if it's '?' or
not.
| ColId Op
{
if (strcmp("?", $2))
ereport(ERROR,
errcode(ERRCODE_SYNTAX_ERROR),
errmsg("unsupported quantifier"),
parser_errposition(@2));
$$ = (Node *)
makeSimpleA_Expr(AEXPR_OP, "?", (Node *)makeString($1), NULL, @1);
}
Another bug was with executor (nodeWindowAgg.c). The code to check
greedy quantifiers missed the case '?'.
> 24 - 0007
>
> I don’t see negative tests for unsupported quantifiers, like PATTERN (A+?).
I will add such test cases.
> 25 - 0007
> ```
> +-- basic test with none greedy pattern
> ```
>
> Typo: “none greedy” -> non-greedy
Will fix.
> No comment for 0008.
Thanks again for the review. I will post v35 patch set soon. Attached
is the diff from v34.
Best regards,
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
diff --git a/doc/src/sgml/advanced.sgml b/doc/src/sgml/advanced.sgml
index 7514f2a3848..eec2a0a9346 100644
--- a/doc/src/sgml/advanced.sgml
+++ b/doc/src/sgml/advanced.sgml
@@ -559,13 +559,14 @@ DEFINE
DOWN AS price < PREV(price)
</programlisting>
- Note that <function>PREV</function> returns the price column in the
- previous row if it's called in a context of row pattern recognition. Thus
- in the second line the definition variable "UP" is <literal>TRUE</literal>
- when the price column in the current row is greater than the price column
- in the previous row. Likewise, "DOWN" is <literal>TRUE</literal> when the
- price column in the current row is lower than the price column in the
- previous row.
+ Note that <function>PREV</function> returns the <literal>price</literal>
+ column in the previous row if it's called in a context of row pattern
+ recognition. Thus in the second line the definition variable "UP"
+ is <literal>TRUE</literal> when the price column in the current row is
+ greater than the price column in the previous row. Likewise, "DOWN"
+ is <literal>TRUE</literal> when the
+ <literal>price</literal> column in the current row is lower than
+ the <literal>price</literal> column in the previous row.
</para>
<para>
Once <literal>DEFINE</literal> exists, <literal>PATTERN</literal> can be
@@ -624,10 +625,10 @@ FROM stock
<para>
When a query involves multiple window functions, it is possible to write
out each one with a separate <literal>OVER</literal> clause, but this is
- duplicative and error-prone if the same windowing behavior is wanted
- for several functions. Instead, each windowing behavior can be named
- in a <literal>WINDOW</literal> clause and then referenced in
<literal>OVER</literal>.
- For example:
+ duplicative and error-prone if the same windowing behavior is wanted for
+ several functions. Instead, each windowing behavior can be named in
+ a <literal>WINDOW</literal> clause and then referenced
+ in <literal>OVER</literal>. For example:
<programlisting>
SELECT sum(salary) OVER w, avg(salary) OVER w
diff --git a/doc/src/sgml/ref/select.sgml b/doc/src/sgml/ref/select.sgml
index 428bd4f7372..aebc797545e 100644
--- a/doc/src/sgml/ref/select.sgml
+++ b/doc/src/sgml/ref/select.sgml
@@ -937,7 +937,6 @@ WINDOW <replaceable
class="parameter">window_name</replaceable> AS ( <replaceabl
[ PARTITION BY <replaceable class="parameter">expression</replaceable> [, ...]
]
[ ORDER BY <replaceable class="parameter">expression</replaceable> [ ASC |
DESC | USING <replaceable class="parameter">operator</replaceable> ] [ NULLS {
FIRST | LAST } ] [, ...] ]
[ <replaceable class="parameter">frame_clause</replaceable> ]
-[ <replaceable class="parameter">row_pattern_common_syntax</replaceable> ]
</synopsis>
</para>
@@ -1097,8 +1096,9 @@ EXCLUDE NO OTHERS
includes following subclauses.
<synopsis>
-[ AFTER MATCH SKIP PAST LAST ROW | AFTER MATCH SKIP TO NEXT ROW ]
-PATTERN <replaceable class="parameter">pattern_variable_name</replaceable>[+]
[, ...]
+[ { AFTER MATCH SKIP PAST LAST ROW | AFTER MATCH SKIP TO NEXT ROW } ]
+[ INITIAL | SEEK ]
+PATTERN ( <replaceable
class="parameter">pattern_variable_name</replaceable>[*|+|?] [, ...] )
DEFINE <replaceable class="parameter">definition_varible_name</replaceable> AS
<replaceable class="parameter">expression</replaceable> [, ...]
</synopsis>
<literal>AFTER MATCH SKIP PAST LAST ROW</literal> or <literal>AFTER MATCH
@@ -1107,9 +1107,16 @@ DEFINE <replaceable
class="parameter">definition_varible_name</replaceable> AS <
ROW</literal> (the default) next row position is next to the last row of
previous match. On the other hand, with <literal>AFTER MATCH SKIP TO NEXT
ROW</literal> next row position is always next to the last row of previous
- match. <literal>DEFINE</literal> defines definition variables along with a
- boolean expression. <literal>PATTERN</literal> defines a sequence of rows
- that satisfies certain conditions using variables defined
+ match. <literal>INITIAL</literal> or <literal>SEEK</literal> defines how a
+ successful pattern matching starts from which row in a
+ frame. If <literal>INITIAL</literal> is specified, the match must start
+ from the first row in the frame. If <literal>SEEK</literal> is specified,
+ the set of matching rows do not necessarily start from the first row. The
+ default is <literal>INITIAL</literal>. Currently
+ only <literal>INITIAL</literal> is supported. <literal>DEFINE</literal>
+ defines definition variables along with a boolean
+ expression. <literal>PATTERN</literal> defines a sequence of rows that
+ satisfies certain conditions using variables defined
in <literal>DEFINE</literal> clause. If the variable is not defined in
the <literal>DEFINE</literal> clause, it is implicitly assumed following
is defined in the <literal>DEFINE</literal> clause.
diff --git a/src/backend/executor/nodeWindowAgg.c
b/src/backend/executor/nodeWindowAgg.c
index 9ce072434b4..d230f43e607 100644
--- a/src/backend/executor/nodeWindowAgg.c
+++ b/src/backend/executor/nodeWindowAgg.c
@@ -4754,7 +4754,7 @@ update_reduced_frame(WindowObject winobj, int64 pos)
{
char *quantifier = strVal(lfirst(lc1));
- if (*quantifier == '+' || *quantifier == '*')
+ if (*quantifier == '+' || *quantifier == '*' || *quantifier ==
'?')
{
greedy = true;
break;
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index fccc26964a0..fdfe14d54e5 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -16825,7 +16825,21 @@ row_pattern_term:
ColId { $$ = (Node *) makeSimpleA_Expr(AEXPR_OP, "",
(Node *)makeString($1), NULL, @1); }
| ColId '*' { $$ = (Node *)
makeSimpleA_Expr(AEXPR_OP, "*", (Node *)makeString($1), NULL, @1); }
| ColId '+' { $$ = (Node *)
makeSimpleA_Expr(AEXPR_OP, "+", (Node *)makeString($1), NULL, @1); }
- | ColId '?' { $$ = (Node *)
makeSimpleA_Expr(AEXPR_OP, "?", (Node *)makeString($1), NULL, @1); }
+/*
+ * '?' quantifier. We cannot write this directly "ColId '?'" because '?' is
+ * not a "self" token. So we let '?' matches Op first then check if it's '?'
+ * or not.
+ */
+ | ColId Op
+ {
+ if (strcmp("?", $2))
+ ereport(ERROR,
+
errcode(ERRCODE_SYNTAX_ERROR),
+
errmsg("unsupported quantifier"),
+
parser_errposition(@2));
+
+ $$ = (Node *)
makeSimpleA_Expr(AEXPR_OP, "?", (Node *)makeString($1), NULL, @1);
+ }
;
row_pattern_definition_list:
@@ -17982,6 +17996,7 @@ unreserved_keyword:
| DECLARE
| DEFAULTS
| DEFERRED
+ | DEFINE
| DEFINER
| DELETE_P
| DELIMITER
@@ -18402,7 +18417,6 @@ reserved_keyword:
| CURRENT_USER
| DEFAULT
| DEFERRABLE
- | DEFINE
| DESC
| DISTINCT
| DO
diff --git a/src/backend/parser/parse_clause.c
b/src/backend/parser/parse_clause.c
index 3521d2780e2..8e4dc732861 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -3920,7 +3920,7 @@ transformRPR(ParseState *pstate, WindowClause *wc,
WindowDef *windef,
if ((wc->frameOptions & FRAMEOPTION_START_CURRENT_ROW) == 0)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("FRAME must start at current row when
row patttern recognition is used")));
+ errmsg("FRAME must start at current row when
row pattern recognition is used")));
/* Transform AFTER MACH SKIP TO clause */
wc->rpSkipTo = windef->rpCommonSyntax->rpSkipTo;
@@ -3949,7 +3949,7 @@ transformDefineClause(ParseState *pstate, WindowClause
*wc, WindowDef *windef,
List **targetlist)
{
/* DEFINE variable name initials */
- static char *defineVariableInitials = "abcdefghijklmnopqrstuvwxyz";
+ static const char *defineVariableInitials =
"abcdefghijklmnopqrstuvwxyz";
ListCell *lc,
*l;
@@ -3959,7 +3959,7 @@ transformDefineClause(ParseState *pstate, WindowClause
*wc, WindowDef *windef,
List *defineClause;
char *name;
int initialLen;
- int i;
+ int numinitials;
/*
* If Row Definition Common Syntax exists, DEFINE clause must exist.
(the
@@ -4030,8 +4030,12 @@ transformDefineClause(ParseState *pstate, WindowClause
*wc, WindowDef *windef,
* equivalent.
*/
restargets = NIL;
+ numinitials = 0;
+ initialLen = strlen(defineVariableInitials);
foreach(lc, windef->rpCommonSyntax->rpDefs)
{
+ char initial[2];
+
restarget = (ResTarget *) lfirst(lc);
name = restarget->name;
@@ -4071,26 +4075,12 @@ transformDefineClause(ParseState *pstate, WindowClause
*wc, WindowDef *windef,
name),
parser_errposition(pstate,
exprLocation((Node *) r))));
}
- restargets = lappend(restargets, restarget);
- }
- list_free(restargets);
-
- /*
- * Create list of row pattern DEFINE variable name's initial. We assign
- * [a-z] to them (up to 26 variable names are allowed).
- */
- restargets = NIL;
- i = 0;
- initialLen = strlen(defineVariableInitials);
-
- foreach(lc, windef->rpCommonSyntax->rpDefs)
- {
- char initial[2];
- restarget = (ResTarget *) lfirst(lc);
- name = restarget->name;
-
- if (i >= initialLen)
+ /*
+ * Create list of row pattern DEFINE variable name's initial. We
+ * assign [a-z] to them (up to 26 variable names are allowed).
+ */
+ if (numinitials >= initialLen)
{
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
@@ -4099,12 +4089,16 @@ transformDefineClause(ParseState *pstate, WindowClause
*wc, WindowDef *windef,
parser_errposition(pstate,
exprLocation((Node *) restarget))));
}
- initial[0] = defineVariableInitials[i++];
+ initial[0] = defineVariableInitials[numinitials++];
initial[1] = '\0';
wc->defineInitial = lappend(wc->defineInitial,
makeString(pstrdup(initial)));
+
+ restargets = lappend(restargets, restarget);
}
+ list_free(restargets);
+ /* turns a list of ResTarget's into a list of TargetEntry's */
defineClause = transformTargetList(pstate,
windef->rpCommonSyntax->rpDefs,
EXPR_KIND_RPR_DEFINE);
@@ -4120,6 +4114,8 @@ transformDefineClause(ParseState *pstate, WindowClause
*wc, WindowDef *windef,
/*
* transformPatternClause
* Process PATTERN clause and return PATTERN clause in the raw
parse tree
+ *
+ * windef->rpCommonSyntax must exist.
*/
static void
transformPatternClause(ParseState *pstate, WindowClause *wc,
@@ -4127,11 +4123,7 @@ transformPatternClause(ParseState *pstate, WindowClause
*wc,
{
ListCell *lc;
- /*
- * Row Pattern Common Syntax clause exists?
- */
- if (windef->rpCommonSyntax == NULL)
- return;
+ Assert(windef->rpCommonSyntax != NULL);
wc->patternVariable = NIL;
wc->patternRegexp = NIL;
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 9e17abbaec5..3cc0ce720b2 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -1621,7 +1621,7 @@ typedef struct WindowClause
Index winref; /* ID referenced by window
functions */
/* did we copy orderClause from refname? */
bool copiedOrder pg_node_attr(query_jumble_ignore);
- /* Row Pattern AFTER MACH SKIP clause */
+ /* Row Pattern AFTER MATCH SKIP clause */
RPSkipTo rpSkipTo; /* Row Pattern Skip To type */
bool initial; /* true if <row pattern initial
or seek> is
* initial */
diff --git a/src/include/parser/kwlist.h b/src/include/parser/kwlist.h
index 7c60b9b44a8..89dc2a4b95a 100644
--- a/src/include/parser/kwlist.h
+++ b/src/include/parser/kwlist.h
@@ -129,7 +129,7 @@ PG_KEYWORD("default", DEFAULT, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("defaults", DEFAULTS, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("deferrable", DEFERRABLE, RESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("deferred", DEFERRED, UNRESERVED_KEYWORD, BARE_LABEL)
-PG_KEYWORD("define", DEFINE, RESERVED_KEYWORD, BARE_LABEL)
+PG_KEYWORD("define", DEFINE, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("definer", DEFINER, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("delete", DELETE_P, UNRESERVED_KEYWORD, BARE_LABEL)
PG_KEYWORD("delimiter", DELIMITER, UNRESERVED_KEYWORD, BARE_LABEL)
diff --git a/src/test/regress/expected/rpr.out
b/src/test/regress/expected/rpr.out
index 59cfed180e7..312b62fb489 100644
--- a/src/test/regress/expected/rpr.out
+++ b/src/test/regress/expected/rpr.out
@@ -165,7 +165,45 @@ SELECT company, tdate, price, first_value(price) OVER w,
last_value(price) OVER
company2 | 07-10-2023 | 1300 | | |
(20 rows)
--- basic test with none greedy pattern
+-- basic test using PREV. Use '?'
+SELECT company, tdate, price, first_value(price) OVER w, last_value(price)
OVER w,
+ nth_value(tdate, 2) OVER w AS nth_second
+ FROM stock
+ WINDOW w AS (
+ PARTITION BY company
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ INITIAL
+ PATTERN (START UP? DOWN+)
+ DEFINE
+ START AS TRUE,
+ UP AS price > PREV(price),
+ DOWN AS price < PREV(price)
+);
+ company | tdate | price | first_value | last_value | nth_second
+----------+------------+-------+-------------+------------+------------
+ company1 | 07-01-2023 | 100 | 100 | 140 | 07-02-2023
+ company1 | 07-02-2023 | 200 | | |
+ company1 | 07-03-2023 | 150 | | |
+ company1 | 07-04-2023 | 140 | | |
+ company1 | 07-05-2023 | 150 | 150 | 90 | 07-06-2023
+ company1 | 07-06-2023 | 90 | | |
+ company1 | 07-07-2023 | 110 | 110 | 120 | 07-08-2023
+ company1 | 07-08-2023 | 130 | | |
+ company1 | 07-09-2023 | 120 | | |
+ company1 | 07-10-2023 | 130 | | |
+ company2 | 07-01-2023 | 50 | 50 | 1400 | 07-02-2023
+ company2 | 07-02-2023 | 2000 | | |
+ company2 | 07-03-2023 | 1500 | | |
+ company2 | 07-04-2023 | 1400 | | |
+ company2 | 07-05-2023 | 1500 | 1500 | 60 | 07-06-2023
+ company2 | 07-06-2023 | 60 | | |
+ company2 | 07-07-2023 | 1100 | 1100 | 1200 | 07-08-2023
+ company2 | 07-08-2023 | 1300 | | |
+ company2 | 07-09-2023 | 1200 | | |
+ company2 | 07-10-2023 | 1300 | | |
+(20 rows)
+
+-- basic test with none-greedy pattern
SELECT company, tdate, price, count(*) OVER w
FROM stock
WINDOW w AS (
@@ -927,7 +965,7 @@ SELECT company, tdate, price, first_value(price) OVER w,
last_value(price) OVER
ERROR: aggregate functions are not allowed in DEFINE
LINE 11: LOWPRICE AS price < count(*)
^
--- FRAME must start at current row when row patttern recognition is used
+-- FRAME must start at current row when row pattern recognition is used
SELECT company, tdate, price, first_value(price) OVER w, last_value(price)
OVER w
FROM stock
WINDOW w AS (
@@ -941,7 +979,7 @@ SELECT company, tdate, price, first_value(price) OVER w,
last_value(price) OVER
UP AS price > PREV(price),
DOWN AS price < PREV(price)
);
-ERROR: FRAME must start at current row when row patttern recognition is used
+ERROR: FRAME must start at current row when row pattern recognition is used
-- SEEK is not supported
SELECT company, tdate, price, first_value(price) OVER w, last_value(price)
OVER w
FROM stock
@@ -977,3 +1015,38 @@ SELECT company, tdate, price, first_value(price) OVER w,
last_value(price) OVER
DOWN AS price < PREV(1)
);
ERROR: row pattern navigation operation's argument must include at least one
column reference
+-- Unsupported quantifier
+SELECT company, tdate, price, first_value(price) OVER w, last_value(price)
OVER w
+ FROM stock
+ WINDOW w AS (
+ PARTITION BY company
+ ORDER BY tdate
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ AFTER MATCH SKIP TO NEXT ROW
+ INITIAL
+ PATTERN (START UP~ DOWN+)
+ DEFINE
+ START AS TRUE,
+ UP AS price > PREV(1),
+ DOWN AS price < PREV(1)
+);
+ERROR: unsupported quantifier
+LINE 9: PATTERN (START UP~ DOWN+)
+ ^
+SELECT company, tdate, price, first_value(price) OVER w, last_value(price)
OVER w
+ FROM stock
+ WINDOW w AS (
+ PARTITION BY company
+ ORDER BY tdate
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ AFTER MATCH SKIP TO NEXT ROW
+ INITIAL
+ PATTERN (START UP+? DOWN+)
+ DEFINE
+ START AS TRUE,
+ UP AS price > PREV(1),
+ DOWN AS price < PREV(1)
+);
+ERROR: unsupported quantifier
+LINE 9: PATTERN (START UP+? DOWN+)
+ ^
diff --git a/src/test/regress/sql/rpr.sql b/src/test/regress/sql/rpr.sql
index 47e67334994..ffcf5476198 100644
--- a/src/test/regress/sql/rpr.sql
+++ b/src/test/regress/sql/rpr.sql
@@ -75,7 +75,22 @@ SELECT company, tdate, price, first_value(price) OVER w,
last_value(price) OVER
DOWN AS price < PREV(price)
);
--- basic test with none greedy pattern
+-- basic test using PREV. Use '?'
+SELECT company, tdate, price, first_value(price) OVER w, last_value(price)
OVER w,
+ nth_value(tdate, 2) OVER w AS nth_second
+ FROM stock
+ WINDOW w AS (
+ PARTITION BY company
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ INITIAL
+ PATTERN (START UP? DOWN+)
+ DEFINE
+ START AS TRUE,
+ UP AS price > PREV(price),
+ DOWN AS price < PREV(price)
+);
+
+-- basic test with none-greedy pattern
SELECT company, tdate, price, count(*) OVER w
FROM stock
WINDOW w AS (
@@ -438,7 +453,7 @@ SELECT company, tdate, price, first_value(price) OVER w,
last_value(price) OVER
LOWPRICE AS price < count(*)
);
--- FRAME must start at current row when row patttern recognition is used
+-- FRAME must start at current row when row pattern recognition is used
SELECT company, tdate, price, first_value(price) OVER w, last_value(price)
OVER w
FROM stock
WINDOW w AS (
@@ -484,3 +499,34 @@ SELECT company, tdate, price, first_value(price) OVER w,
last_value(price) OVER
UP AS price > PREV(1),
DOWN AS price < PREV(1)
);
+
+-- Unsupported quantifier
+SELECT company, tdate, price, first_value(price) OVER w, last_value(price)
OVER w
+ FROM stock
+ WINDOW w AS (
+ PARTITION BY company
+ ORDER BY tdate
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ AFTER MATCH SKIP TO NEXT ROW
+ INITIAL
+ PATTERN (START UP~ DOWN+)
+ DEFINE
+ START AS TRUE,
+ UP AS price > PREV(1),
+ DOWN AS price < PREV(1)
+);
+
+SELECT company, tdate, price, first_value(price) OVER w, last_value(price)
OVER w
+ FROM stock
+ WINDOW w AS (
+ PARTITION BY company
+ ORDER BY tdate
+ ROWS BETWEEN CURRENT ROW AND UNBOUNDED FOLLOWING
+ AFTER MATCH SKIP TO NEXT ROW
+ INITIAL
+ PATTERN (START UP+? DOWN+)
+ DEFINE
+ START AS TRUE,
+ UP AS price > PREV(1),
+ DOWN AS price < PREV(1)
+);
