shauryachats commented on code in PR #18425:
URL: https://github.com/apache/pinot/pull/18425#discussion_r3249329277
##########
pinot-common/src/test/java/org/apache/pinot/sql/parsers/CalciteSqlCompilerTest.java:
##########
@@ -879,6 +879,27 @@ public void testQueryOptions() {
} catch (SqlCompilationException e) {
Assert.assertTrue(e.getCause() instanceof ParseException);
}
+
+ // Options inside SQL comments must NOT be honored.
+ // A trailing -- comment should not be mistaken for a real query option.
+ pinotQuery = compileToPinotQuery(
+ "SELECT col1, count(*) FROM foo GROUP BY col1 --
option(skipUpsert=true)");
+ Assert.assertTrue(pinotQuery.getQueryOptions() == null ||
pinotQuery.getQueryOptions().isEmpty(),
+ "option(...) inside a -- comment must not be parsed as a query
option");
+
+ // Same check when the -- comment appears inline after a WHERE predicate
(multi-line query).
+ // The regex finds OPTION() starting from the 'O', ignoring the preceding
'--', so without
+ // the fix this would incorrectly honour skipUpsert.
+ pinotQuery = compileToPinotQuery(
+ "select *\nfrom foo\nwhere uuid = 1 --OPTION(skipUpsert = true)");
+ Assert.assertTrue(pinotQuery.getQueryOptions() == null ||
pinotQuery.getQueryOptions().isEmpty(),
+ "OPTION(...) after -- in a WHERE clause must not be parsed as a query
option");
+
+ // A /* */ block comment should also not trigger the OPTIONS parser.
+ pinotQuery = compileToPinotQuery(
+ "SELECT col1, count(*) FROM foo GROUP BY col1 /*
option(skipUpsert=true) */");
+ Assert.assertTrue(pinotQuery.getQueryOptions() == null ||
pinotQuery.getQueryOptions().isEmpty(),
+ "option(...) inside a /* */ comment must not be parsed as a query
option");
Review Comment:
Can you add a test for double dashes within double quotes? Legally a column
name "my--column--name" is valid under Pinot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
