vvivekiyer commented on code in PR #10315:
URL: https://github.com/apache/pinot/pull/10315#discussion_r1113410216
##########
pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java:
##########
@@ -109,10 +109,17 @@ private static String removeTerminatingSemicolon(String
sql) {
public static SqlNodeAndOptions compileToSqlNodeAndOptions(String sql)
throws SqlCompilationException {
+ return compileToSqlNodeAndOptions(sql, true);
+ }
+
+ public static SqlNodeAndOptions compileToSqlNodeAndOptions(String sql,
boolean isRemoveComments)
+ throws SqlCompilationException {
long parseStartTimeNs = System.nanoTime();
// Remove the comments from the query
Review Comment:
The code for removing comments was added in
https://github.com/apache/pinot/issues/7714 to avoid misusing the older support
for `OPTIONS`. As per my understanding, this logic is not needed anymore
because we have moved to SET based options.
So we can remove this logic entirely?
##########
pinot-query-planner/src/main/java/org/apache/pinot/query/QueryEnvironment.java:
##########
@@ -172,12 +173,12 @@ public String explainQuery(String sqlQuery,
SqlNodeAndOptions sqlNodeAndOptions)
@VisibleForTesting
public QueryPlan planQuery(String sqlQuery) {
Review Comment:
This function is only called from tests, right?
The actual code path is `MultiStageBrokerRequestHandler.handleRequest()` ->
`QueryEnvironment.planQuery(sqlQuery, sqlNodeAndOptions)`.
--
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]
