This is an automated email from the ASF dual-hosted git repository. michaelsmith pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit d2cb00cecea1883955a5bc4be997567bb04be0f8 Author: Steve Carlin <[email protected]> AuthorDate: Wed Oct 16 11:27:48 2024 -0700 IMPALA-13456: Calcite planner: fix issues with quotes Changed the parser to use quotes that are inline with how Impala treats quotes, including allowing single quotes, double quotes, and back ticks for aliases, and also allowing the backslash to be used as an escape character. This is inline with what BigQuery uses in Calcite. A couple of unit tests were added, but these will be tested more extensively by the ParserTest frontend unit test when that gets committed. Also, added the VALUE as a nonreserved keyword which is used in the tpcds queries (along with the doublequotes) Change-Id: I67ebb19912714c240b99a42d9f2f02f78c189350 Reviewed-on: http://gerrit.cloudera.org:8080/21942 Tested-by: Impala Public Jenkins <[email protected]> Reviewed-by: Michael Smith <[email protected]> --- java/calcite-planner/src/main/codegen/config.fmpp | 1 + .../src/main/codegen/templates/Parser.jj | 43 ++++++++++++---------- .../impala/calcite/service/CalciteQueryParser.java | 5 ++- .../queries/QueryTest/calcite.test | 16 ++++++++ 4 files changed, 45 insertions(+), 20 deletions(-) diff --git a/java/calcite-planner/src/main/codegen/config.fmpp b/java/calcite-planner/src/main/codegen/config.fmpp index 0a609de35..18886790a 100644 --- a/java/calcite-planner/src/main/codegen/config.fmpp +++ b/java/calcite-planner/src/main/codegen/config.fmpp @@ -361,6 +361,7 @@ parser: { "UTF16" "UTF32" "UTF8" + "VALUE" "VERSION" "VIEW" "WEEK" diff --git a/java/calcite-planner/src/main/codegen/templates/Parser.jj b/java/calcite-planner/src/main/codegen/templates/Parser.jj index ce018c0ae..6a59388dd 100644 --- a/java/calcite-planner/src/main/codegen/templates/Parser.jj +++ b/java/calcite-planner/src/main/codegen/templates/Parser.jj @@ -1945,12 +1945,8 @@ void AddSelectItem(List<SqlNode> list) : ( [ <AS> ] ( - id = SimpleIdentifier() - | - // Mute the warning about ambiguity between alias and continued - // string literal. - LOOKAHEAD(1) - id = SimpleIdentifierFromStringLiteral() + // Added Impala code to handle when there are quotes and no quotes + id = SimpleIdentifierFromAllQuotesOrNonQuotes() ) { list.add(SqlStdOperatorTable.AS.createCall(span().end(e), e, id)); } | { list.add(e); } @@ -2233,7 +2229,8 @@ SqlNode TableRef3(ExprContext exprContext, boolean lateral) : tableRef = Unpivot(tableRef) ] [ - [ <AS> ] alias = SimpleIdentifier() + // Added Impala code to handle when there are quotes and no quotes + [ <AS> ] alias = SimpleIdentifierFromAllQuotesOrNonQuotes() ( columnAliasList = ParenthesizedSimpleIdentifierList() | { columnAliasList = null; } @@ -3502,7 +3499,8 @@ void AddWithItem(List<SqlWithItem> list, SqlLiteral recursive) : final SqlNode definition; } { - id = SimpleIdentifier() + // Added Impala code to handle when there are quotes and no quotes + id = SimpleIdentifierFromAllQuotesOrNonQuotes() ( columnList = ParenthesizedSimpleIdentifierList() | { columnList = null; } ) <AS> definition = ParenthesizedExpression(ExprContext.ACCEPT_QUERY) @@ -4690,10 +4688,6 @@ String SimpleStringLiteral() : <BIG_QUERY_QUOTED_STRING> { return SqlParserUtil.stripQuotes(token.image, "'", "'", "\\'", Casing.UNCHANGED); } -| - <BIG_QUERY_DOUBLE_QUOTED_STRING> { - return SqlParserUtil.stripQuotes(token.image, DQ, DQ, "\\\"", Casing.UNCHANGED); - } } /** @@ -5437,21 +5431,32 @@ SqlIdentifier SimpleIdentifier() : } } -/** - * Parses a character literal as an SqlIdentifier. - * Only valid for column aliases in certain dialects. - */ -SqlIdentifier SimpleIdentifierFromStringLiteral() : +// Added Impala code to handle when there are quotes and no quotes +SqlIdentifier SimpleIdentifierFromAllQuotesOrNonQuotes() : { + SqlIdentifier stringLiteral; } { - <QUOTED_STRING> { + <BIG_QUERY_DOUBLE_QUOTED_STRING> { if (!this.conformance.allowCharLiteralAlias()) { throw SqlUtil.newContextException(getPos(), RESOURCE.charLiteralAliasNotValid()); } - final String s = SqlParserUtil.parseString(token.image); + String s = SqlParserUtil.stripQuotes(token.image, "\"", "\"", "\\\"", Casing.UNCHANGED); return new SqlIdentifier(s, getPos()); } + | + <BIG_QUERY_QUOTED_STRING> { + if (!this.conformance.allowCharLiteralAlias()) { + throw SqlUtil.newContextException(getPos(), RESOURCE.charLiteralAliasNotValid()); + } + final String t = SqlParserUtil.parseString(token.image); + return new SqlIdentifier(t, getPos()); + } + | + stringLiteral = SimpleIdentifier() + { + return stringLiteral; + } } /** diff --git a/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java b/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java index d5ab5f2ff..718db240d 100644 --- a/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java +++ b/java/calcite-planner/src/main/java/org/apache/impala/calcite/service/CalciteQueryParser.java @@ -17,6 +17,7 @@ package org.apache.impala.calcite.service; +import org.apache.calcite.avatica.util.Quoting; import org.apache.calcite.sql.SqlNode; import org.apache.calcite.sql.parser.SqlParser; import org.apache.calcite.sql.parser.SqlParseException; @@ -44,7 +45,9 @@ public class CalciteQueryParser implements CompilerStep { // Create an SQL parser SqlParser parser = SqlParser.create(queryCtx_.getStmt(), SqlParser.config().withParserFactory(ImpalaSqlParserImpl.FACTORY) - .withConformance(ImpalaConformance.INSTANCE)); + .withConformance(ImpalaConformance.INSTANCE) + .withQuoting(Quoting.BACK_TICK_BACKSLASH) + ); // Parse the query into an AST SqlNode sqlNode = parser.parseQuery(); diff --git a/testdata/workloads/functional-query/queries/QueryTest/calcite.test b/testdata/workloads/functional-query/queries/QueryTest/calcite.test index d33531f65..05a4a8c93 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/calcite.test +++ b/testdata/workloads/functional-query/queries/QueryTest/calcite.test @@ -830,3 +830,19 @@ from functional.alltypes where id < 8 7,1,2,3,4,4,4,4,3,2,1,0,0,0 ---- TYPES INT, BIGINT, BIGINT, BIGINT, BIGINT, BIGINT, BIGINT, BIGINT, BIGINT, BIGINT, BIGINT, BIGINT, BIGINT, BIGINT +==== +---- QUERY +# make sure this parses +select '1' as 'hello'; +---- RESULTS +'1' +---- TYPES +string +==== +---- QUERY +# make sure this parses +select "1" as "hello"; +---- RESULTS +'1' +---- TYPES +string
