siddharthteotia commented on code in PR #8484: URL: https://github.com/apache/pinot/pull/8484#discussion_r849673812
########## pinot-common/src/main/java/org/apache/pinot/sql/parsers/CalciteSqlParser.java: ########## @@ -311,16 +301,29 @@ public static Set<String> extractIdentifiers(List<Expression> expressions, boole * @throws SqlCompilationException if String is not a valid expression. */ public static Expression compileToExpression(String expression) { - SqlParser sqlParser = SqlParser.create(expression, PARSER_CONFIG); SqlNode sqlNode; - try { - sqlNode = sqlParser.parseExpression(); - } catch (SqlParseException e) { + try (StringReader inStream = new StringReader(expression)) { + SqlParserImpl sqlParser = newSqlParser(inStream); + sqlNode = sqlParser.parseSqlExpressionEof(); + } catch (Throwable e) { throw new SqlCompilationException("Caught exception while parsing expression: " + expression, e); } return toExpression(sqlNode); } + @VisibleForTesting + static SqlParserImpl newSqlParser(StringReader inStream) { + SqlParserImpl sqlParser = new SqlParserImpl(inStream); Review Comment: IIUC, the current code `SqlParser sqlParser = SqlParser.create(expression, PARSER_CONFIG);` will return SqlBabelParserImpl (created from SqlBabelParserImplFactory). In this PR, I think we are going to use SqlParserImpl (SqlParserImplFactory) and that is going to give the same behavior as today because `SqlConformance` is still set to BABEL ? -- 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: commits-unsubscr...@pinot.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org