This is an automated email from the ASF dual-hosted git repository.
rongr pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new 080e849a6c fixing REGEX OPTION parser (#8905)
080e849a6c is described below
commit 080e849a6c68da8beb78ce9c9f943d7f5a8a8f78
Author: Rong Rong <[email protected]>
AuthorDate: Thu Jun 16 13:06:09 2022 -0700
fixing REGEX OPTION parser (#8905)
* fixing REGEX OPTION parser.
New syntax:
1. only allow a single OPTION keyword
2. only allow OPTION at end of file
Co-authored-by: Rong Rong <[email protected]>
---
.../apache/pinot/sql/parsers/CalciteSqlParser.java | 8 ++------
.../pinot/sql/parsers/CalciteSqlCompilerTest.java | 24 ++++++++++++++--------
.../pinot/sql/parsers/dml/InsertIntoFileTest.java | 10 ++++-----
3 files changed, 23 insertions(+), 19 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
index f725b9fbca..90a9863beb 100644
---
a/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
+++
b/pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java
@@ -81,13 +81,9 @@ public class CalciteSqlParser {
// PQL syntax is: `OPTION (<key> = <value>)`
//
// Multiple OPTIONs is also supported by:
- // either
- // `OPTION (<k1> = <v1>, <k2> = <v2>, <k3> = <v3>)`
- // or
- // `OPTION (<k1> = <v1>) OPTION (<k2> = <v2>) OPTION (<k3> = <v3>)`
- // TODO: move to use parser syntax extension: `OPTION` `(` `<key>` =
`<value>` [, `<key>` = `<value>`]* `)`
+ // `OPTION (<k1> = <v1>, <k2> = <v2>, <k3> = <v3>)`
private static final Pattern OPTIONS_REGEX_PATTEN =
- Pattern.compile("option\\s*\\(([^\\)]+)\\)", Pattern.CASE_INSENSITIVE);
+ Pattern.compile("\\s*option\\s*\\(([^\\)]+)\\)\\s*\\Z",
Pattern.CASE_INSENSITIVE);
/**
* Checks for the presence of semicolon in the sql query and modifies the
query accordingly
diff --git
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
index 5060085fe9..8cbd47fa32 100644
---
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java
@@ -34,6 +34,7 @@ import org.apache.pinot.common.request.Literal;
import org.apache.pinot.common.request.PinotQuery;
import org.apache.pinot.segment.spi.AggregationFunctionType;
import org.apache.pinot.sql.FilterKind;
+import org.apache.pinot.sql.parsers.parser.ParseException;
import org.apache.pinot.sql.parsers.parser.SqlInsertFromFile;
import org.apache.pinot.sql.parsers.parser.SqlParserImpl;
import org.apache.pinot.sql.parsers.rewriter.CompileTimeFunctionsInvoker;
@@ -711,14 +712,21 @@ public class CalciteSqlCompilerTest {
Assert.assertEquals(pinotQuery.getQueryOptions().get("foo"), "1234");
Assert.assertEquals(pinotQuery.getQueryOptions().get("bar"), "'potato'");
- pinotQuery = CalciteSqlParser.compileToPinotQuery(
- "select * from vegetables where name <> 'Brussels sprouts' OPTION
(delicious=yes) option(foo=1234) option"
- + "(bar='potato')");
- Assert.assertEquals(pinotQuery.getQueryOptionsSize(), 3);
- Assert.assertTrue(pinotQuery.getQueryOptions().containsKey("delicious"));
- Assert.assertEquals(pinotQuery.getQueryOptions().get("delicious"), "yes");
- Assert.assertEquals(pinotQuery.getQueryOptions().get("foo"), "1234");
- Assert.assertEquals(pinotQuery.getQueryOptions().get("bar"), "'potato'");
+ // Assert that wrongly inserted query option will not be parsed.
+ try {
+ CalciteSqlParser.compileToPinotQuery(
+ "select * from vegetables where name <> 'Brussels sprouts' OPTION
(delicious=yes) option(foo=1234) option"
+ + "(bar='potato')");
+ } catch (SqlCompilationException e) {
+ Assert.assertTrue(e.getCause() instanceof ParseException);
+ Assert.assertTrue(e.getCause().getMessage().contains("OPTION"));
+ }
+ try {
+ CalciteSqlParser.compileToPinotQuery(
+ "select * from vegetables where name <> 'Brussels OPTION
(delicious=yes)");
+ } catch (SqlCompilationException e) {
+ Assert.assertTrue(e.getCause() instanceof ParseException);
+ }
}
@Test
diff --git
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/dml/InsertIntoFileTest.java
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/dml/InsertIntoFileTest.java
index ef688d32b5..2092f1b18f 100644
---
a/pinot-common/src/test/java/org/apache/pinot/sql/parsers/dml/InsertIntoFileTest.java
+++
b/pinot-common/src/test/java/org/apache/pinot/sql/parsers/dml/InsertIntoFileTest.java
@@ -32,11 +32,11 @@ public class InsertIntoFileTest {
throws Exception {
String insertIntoSql = "INSERT INTO \"baseballStats\"\n"
+ "FROM FILE 's3://my-bucket/path/to/data/'\n"
- + "OPTION(taskName=myTask-1)\n"
- +
"OPTION(input.fs.className=org.apache.pinot.plugin.filesystem.S3PinotFS)\n"
- + "OPTION(input.fs.prop.accessKey=my-access-key)\n"
- + "OPTION(input.fs.prop.secretKey=my-secret-key)\n"
- + "OPTION(input.fs.prop.region=us-west-2)";
+ + "OPTION(taskName=myTask-1,"
+ + "input.fs.className=org.apache.pinot.plugin.filesystem.S3PinotFS,"
+ + "input.fs.prop.accessKey=my-access-key,"
+ + "input.fs.prop.secretKey=my-secret-key,"
+ + "input.fs.prop.region=us-west-2)";
InsertIntoFile insertIntoFile =
InsertIntoFile.parse(CalciteSqlParser.compileToSqlNodeAndOptions(insertIntoSql));
Assert.assertEquals(insertIntoFile.getTable(), "baseballStats");
Assert.assertEquals(insertIntoFile.getExecutionType(),
DataManipulationStatement.ExecutionType.MINION);
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
