Thread
-
Re: Row pattern recognition
Tatsuo Ishii <ishii@postgresql.org> — 2025-12-01T12:42:18Z
>> 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. Attached are the v35 patches for Row pattern recognition. Chao Li reviewed v34 thoroughly. Thank you! v35 reflects the review comments. Major differences from v34 include: - Make "DEFINE" an unreserved keyword. Previously it was a reserved keyword. - Refactor transformDefineClause() to make two foreach loops into single foreach loop. - Make '?' quantifier in PATTERN work as advertised. Test for '?' quantifier is added too. - Unsupported quantifier test added. - Fix get_rule_define(). - Fix memory leak related to regcomp. - Move regcomp compiled result cache from static data to WindowAggState. - Fix several typos. Best regards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp