Skip to content

Commit

Permalink
[CALCITE-2993] ParseException may be thrown for legal SQL queries due…
Browse files Browse the repository at this point in the history
… to incorrect "LOOKAHEAD(1)" hints

Also:
* Following [CALCITE-883], add absent non-reserved keywords to config.fmpp files;
* Remove redundant "LOOKAHEAD(1)" hints;
* The case "select json_object(key: value) from t" is added to unit test but not fixed. See the reason from method "SqlParserTest#testJsonObject".
  • Loading branch information
zhztheplayer committed Apr 23, 2019
1 parent 02eb106 commit 577e72c
Show file tree
Hide file tree
Showing 6 changed files with 66 additions and 6 deletions.
2 changes: 2 additions & 0 deletions babel/src/main/codegen/config.fmpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ data: {
"GOTO"
"GRANTED"
"HIERARCHY"
"IGNORE"
"IMMEDIATE"
"IMMEDIATELY"
"IMPLEMENTATION"
Expand Down Expand Up @@ -210,6 +211,7 @@ data: {
"RELATIVE"
"REPEATABLE"
"REPLACE"
"RESPECT"
"RESTART"
"RESTRICT"
"RETURNED_CARDINALITY"
Expand Down
19 changes: 14 additions & 5 deletions core/src/main/codegen/templates/Parser.jj
Original file line number Diff line number Diff line change
Expand Up @@ -1894,7 +1894,7 @@ SqlNode TableRef2(boolean lateral) :
tableRef = unnestOp.createCall(s.end(this), args.toArray());
}
|
[ LOOKAHEAD(1) <LATERAL> { lateral = true; } ]
[<LATERAL> { lateral = true; } ]
<TABLE> { s = span(); } <LPAREN>
tableRef = TableFunctionCall(s.pos())
<RPAREN>
Expand Down Expand Up @@ -2597,7 +2597,17 @@ SqlMatchRecognize MatchRecognize(SqlNode tableRef) :
s1.end(var), var);
}
|
[ LOOKAHEAD(1) <LAST> ] var = SimpleIdentifier() {
// This "LOOKAHEAD({true})" is a workaround for Babel.
// Because of babel parser uses option "LOOKAHEAD=2" globally,
// JavaCC generates something like "LOOKAHEAD(2, [<LAST>] SimpleIdentifier())"
// here. But the correct LOOKAHEAD should be
// "LOOKAHEAD(2, [ LOOKAHEAD(2, <LAST> SimpleIdentifier()) <LAST> ]
// SimpleIdentifier())" which have the syntactic lookahead for <LAST> considered.
//
// Overall LOOKAHEAD({true}) is even better as this is the last branch in the
// choice.
LOOKAHEAD({true})
[ LOOKAHEAD(2, <LAST> SimpleIdentifier()) <LAST> ] var = SimpleIdentifier() {
after = SqlMatchRecognize.SKIP_TO_LAST.createCall(
s1.end(var), var);
}
Expand Down Expand Up @@ -3423,7 +3433,6 @@ SqlNode AtomicRowExpression() :
}
{
(
LOOKAHEAD(1)
e = Literal()
|
e = DynamicParam()
Expand Down Expand Up @@ -5221,7 +5230,7 @@ List<SqlNode> JsonNameAndValue() :
}
{
[
LOOKAHEAD(1)
LOOKAHEAD(2, <KEY> JsonName())
<KEY> { kvMode = true; }
]
e = JsonName() {
Expand Down Expand Up @@ -5664,7 +5673,7 @@ SqlNode NamedFunctionCall() :
call = createCall(qualifiedName, s.end(this), funcType, quantifier, args);
}
[
LOOKAHEAD(1) call = nullTreatment(call)
LOOKAHEAD(2) call = nullTreatment(call)
]
[
call = withinGroup(call)
Expand Down
2 changes: 2 additions & 0 deletions core/src/test/codegen/config.fmpp
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ data: {
"GOTO"
"GRANTED"
"HIERARCHY"
"IGNORE"
"IMMEDIATE"
"IMMEDIATELY"
"IMPLEMENTATION"
Expand Down Expand Up @@ -214,6 +215,7 @@ data: {
"RELATIVE"
"REPEATABLE"
"REPLACE"
"RESPECT"
"RESTART"
"RESTRICT"
"RETURNED_CARDINALITY"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4276,6 +4276,15 @@ void checkPeriodPredicate(Checker checker) {
+ "ORDER BY `COL1`\n"
+ "FETCH NEXT 10 ROWS ONLY";
sql(sql).ok(expected);

// See [CALCITE-2993] ParseException may be thrown for legal
// SQL queries due to incorrect "LOOKAHEAD(1)" hints
sql("select lead(x) ignore from t")
.ok("SELECT LEAD(`X`) AS `IGNORE`\n"
+ "FROM `T`");
sql("select lead(x) respect from t")
.ok("SELECT LEAD(`X`) AS `RESPECT`\n"
+ "FROM `T`");
}

@Test public void testAs() {
Expand Down Expand Up @@ -8142,6 +8151,31 @@ public void subTestIntervalSecondFailsValidation() {
sql(sql).ok(expected);
}

/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-2993">[CALCITE-2993]
* ParseException may be thrown for legal SQL queries due to incorrect
* "LOOKAHEAD(1)" hints</a>. */
@Test public void testMatchRecognizePatternSkip6() {
final String sql = "select *\n"
+ " from t match_recognize\n"
+ " (\n"
+ " after match skip to last\n"
+ " pattern (strt down+ up+)\n"
+ " define\n"
+ " down as down.price < PREV(down.price),\n"
+ " up as up.price > prev(up.price)\n"
+ " ) mr";
final String expected = "SELECT *\n"
+ "FROM `T` MATCH_RECOGNIZE(\n"
+ "AFTER MATCH SKIP TO LAST `LAST`\n"
+ "PATTERN (((`STRT` (`DOWN` +)) (`UP` +)))\n"
+ "DEFINE "
+ "`DOWN` AS (`DOWN`.`PRICE` < PREV(`DOWN`.`PRICE`, 1)), "
+ "`UP` AS (`UP`.`PRICE` > PREV(`UP`.`PRICE`, 1))"
+ ") AS `MR`";
sql(sql).ok(expected);
}

@Test public void testMatchRecognizeSubset1() {
final String sql = "select *\n"
+ " from t match_recognize\n"
Expand Down Expand Up @@ -8454,6 +8488,18 @@ public void subTestIntervalSecondFailsValidation() {
"JSON_OBJECT(KEY 'foo' VALUE "
+ "JSON_OBJECT(KEY 'foo' VALUE 'bar' NULL ON NULL) "
+ "FORMAT JSON NULL ON NULL)");

if (!Bug.TODO_FIXED) {
return;
}
// "LOOKAHEAD(2) list = JsonNameAndValue()" does not generate
// valid LOOKAHEAD codes for the case "key: value".
//
// You can see the generated codes that are located at method
// SqlParserImpl#JsonObjectFunctionCall. Looking ahead fails
// immediately after seeking the tokens <KEY> and <COLON>.
checkExp("json_object(key: value)",
"JSON_OBJECT(KEY `KEY` VALUE `VALUE` NULL ON NULL)");
}

@Test public void testJsonType() {
Expand Down
1 change: 0 additions & 1 deletion piglet/src/main/javacc/PigletParser.jj
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,6 @@ Assignment foreachStmt(final Identifier target) :
{
<FOREACH> id = simpleIdentifier()
(
LOOKAHEAD(1)
<GENERATE> expList = expCommaList() <SEMICOLON> {
return new ForeachStmt(pos3(target), target, id, expList, schema);
}
Expand Down
2 changes: 2 additions & 0 deletions server/src/main/codegen/config.fmpp
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ data: {
"GOTO"
"GRANTED"
"HIERARCHY"
"IGNORE"
"IMMEDIATE"
"IMMEDIATELY"
"IMPLEMENTATION"
Expand Down Expand Up @@ -222,6 +223,7 @@ data: {
"RELATIVE"
"REPEATABLE"
"REPLACE"
"RESPECT"
"RESTART"
"RESTRICT"
"RETURNED_CARDINALITY"
Expand Down

0 comments on commit 577e72c

Please sign in to comment.