Re: Row pattern recognition

> 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