From 920830a61b38cb7020082ac4ce1f0787b39e6ba1 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Sun, 20 Oct 2019 19:15:45 -0700 Subject: [PATCH 01/27] Added an unquoter utils class to extract string text from the back-ticks --- .../sql/utils/BackticksUnquoter.java | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/BackticksUnquoter.java diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/BackticksUnquoter.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/BackticksUnquoter.java new file mode 100644 index 0000000000..1d921db144 --- /dev/null +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/BackticksUnquoter.java @@ -0,0 +1,62 @@ +/* + * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package com.amazon.opendistroforelasticsearch.sql.utils; + +/** + * Utils Class for remove the back-ticks of identifiers + */ +public class BackticksUnquoter { + + /** + * + * @param text + * @return An unquoted string whose outer pair of back-ticks (if any) has been removed + */ + public static String unquoteSingleField(String text) { + if (text != null && text.startsWith("`") && text.endsWith("`")) { + return text.substring(1, text.length() - 1); + } + return text; + } + + /** + * + * @param text + * @return A string whose each dot-seperated field has been unquoted from back-ticks (if any) + */ + public static String unquoteFullColumn(String text) { + if (text == null) { + return null; + } + String[] strs = text.split("\\."); + for (int i = 0; i < strs.length; i++) { + String unquotedSubstr = strs[i]; + if (unquotedSubstr != null && unquotedSubstr.startsWith("`") && unquotedSubstr.endsWith("`")) { + unquotedSubstr = strs[i].substring(1, strs[i].length() - 1); + } + strs[i] = unquotedSubstr; + } + StringBuilder unquotedStrBuilder = new StringBuilder(strs[0]); + for (int i = 1; i < strs.length; i++) { + unquotedStrBuilder.append(".").append(strs[i]); + } + return unquotedStrBuilder.toString(); + } + + public BackticksUnquoter() { + } + +} From 3e3a91e8b4b293c95d9f4874cbb255ee67e4aec6 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Sun, 20 Oct 2019 19:18:21 -0700 Subject: [PATCH 02/27] Modified the semantic analyzer to visit the index names and field names that have been unquoted from the back-ticks --- .../antlr/semantic/visitor/SemanticAnalyzer.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java index 699bd29a40..e8acb5daea 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java @@ -17,6 +17,7 @@ import com.amazon.opendistroforelasticsearch.sql.antlr.semantic.types.Type; import com.amazon.opendistroforelasticsearch.sql.antlr.visitor.GenericSqlParseTreeVisitor; +import com.amazon.opendistroforelasticsearch.sql.utils.BackticksUnquoter; import java.util.List; @@ -60,20 +61,20 @@ public Type visitSelect(List itemTypes) { @Override public void visitAs(String alias, Type type) { - mappingLoader.visitAs(alias, type); - typeChecker.visitAs(alias, type); + mappingLoader.visitAs(new BackticksUnquoter().unquoteSingleField(alias), type); + typeChecker.visitAs(new BackticksUnquoter().unquoteSingleField(alias), type); } @Override public Type visitIndexName(String indexName) { - mappingLoader.visitIndexName(indexName); - return typeChecker.visitIndexName(indexName); + mappingLoader.visitIndexName(new BackticksUnquoter().unquoteSingleField(indexName)); + return typeChecker.visitIndexName(new BackticksUnquoter().unquoteSingleField(indexName)); } @Override public Type visitFieldName(String fieldName) { - mappingLoader.visitFieldName(fieldName); - return typeChecker.visitFieldName(fieldName); + mappingLoader.visitFieldName(new BackticksUnquoter().unquoteFullColumn(fieldName)); + return typeChecker.visitFieldName(new BackticksUnquoter().unquoteFullColumn(fieldName)); } @Override From 5ef2ba83eb1d1ea332ef288d70db2c50168e7cc3 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Sun, 20 Oct 2019 19:20:41 -0700 Subject: [PATCH 03/27] Added an UnquoteIdentifierRule rewrite the AST that are constructed from the druid parser --- .../sql/query/ESActionFactory.java | 2 + .../sql/rewriter/UnquoteIdentifierRule.java | 90 +++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/UnquoteIdentifierRule.java diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/ESActionFactory.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/ESActionFactory.java index 733b0815b1..79707421e2 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/ESActionFactory.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/ESActionFactory.java @@ -41,6 +41,7 @@ import com.amazon.opendistroforelasticsearch.sql.query.multi.MultiQueryAction; import com.amazon.opendistroforelasticsearch.sql.query.multi.MultiQuerySelect; import com.amazon.opendistroforelasticsearch.sql.rewriter.RewriteRuleExecutor; +import com.amazon.opendistroforelasticsearch.sql.rewriter.UnquoteIdentifierRule; import com.amazon.opendistroforelasticsearch.sql.rewriter.alias.TableAliasPrefixRemoveRule; import com.amazon.opendistroforelasticsearch.sql.rewriter.matchtoterm.TermFieldRewriter; import com.amazon.opendistroforelasticsearch.sql.rewriter.matchtoterm.TermFieldRewriter.TermRewriterFilter; @@ -78,6 +79,7 @@ public static QueryAction create(Client client, String sql) throws SqlParseExcep RewriteRuleExecutor ruleExecutor = RewriteRuleExecutor.builder() .withRule(new SQLExprParentSetterRule()) + .withRule(new UnquoteIdentifierRule()) .withRule(new TableAliasPrefixRemoveRule()) .withRule(new SubQueryRewriteRule()) .build(); diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/UnquoteIdentifierRule.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/UnquoteIdentifierRule.java new file mode 100644 index 0000000000..a82ca1d43b --- /dev/null +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/UnquoteIdentifierRule.java @@ -0,0 +1,90 @@ +/* + * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package com.amazon.opendistroforelasticsearch.sql.rewriter; + +import com.alibaba.druid.sql.ast.expr.SQLIdentifierExpr; +import com.alibaba.druid.sql.ast.expr.SQLPropertyExpr; +import com.alibaba.druid.sql.ast.expr.SQLQueryExpr; +import com.alibaba.druid.sql.ast.statement.SQLExprTableSource; +import com.alibaba.druid.sql.ast.statement.SQLSelectItem; +import com.alibaba.druid.sql.dialect.mysql.visitor.MySqlASTVisitorAdapter; +import com.amazon.opendistroforelasticsearch.sql.utils.BackticksUnquoter; + +public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements RewriteRule { + + String identifier = null; + + @Override + public boolean visit(SQLPropertyExpr propertyExpr) { + String identifier = ((SQLIdentifierExpr) propertyExpr.getOwner()).getName(); + if (identifier.startsWith("`") && identifier.endsWith("`")) { + this.identifier = new BackticksUnquoter().unquoteSingleField(identifier) + "." + + new BackticksUnquoter().unquoteSingleField(propertyExpr.getName()); + SQLSelectItem selectItem = (SQLSelectItem) propertyExpr.getParent(); + selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); + this.identifier = null; + return false; + } + return true; + } + + @Override + public boolean visit(SQLSelectItem selectItem) { + try { + String identifier = ((SQLIdentifierExpr) selectItem.getExpr()).getName(); + if (identifier.endsWith(".")) { + this.identifier = identifier + new BackticksUnquoter().unquoteSingleField(selectItem.getAlias()); + selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); + selectItem.setAlias(null); + } + } finally { + return true; + } + } + + @Override + public boolean visit(SQLIdentifierExpr identifierExpr) { + if (identifier != null) { + return false; + } + return true; + } + + @Override + public void endVisit(SQLIdentifierExpr identifierExpr) { + if (identifier != null) { + identifierExpr.setName(identifier); + identifier = null; + } else { + identifierExpr.setName(new BackticksUnquoter().unquoteFullColumn(identifierExpr.getName())); + } + } + + @Override + public void endVisit(SQLExprTableSource tableSource) { + tableSource.setAlias(new BackticksUnquoter().unquoteSingleField(tableSource.getAlias())); + } + + @Override + public boolean match(SQLQueryExpr root) { + return true; + } + + @Override + public void rewrite(SQLQueryExpr root) { + root.accept(new UnquoteIdentifierRule()); + } +} From a91696bcdb662a09d1e30c56e3431c534b03a1da Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Sun, 20 Oct 2019 19:35:33 -0700 Subject: [PATCH 04/27] Moved the rule into "identifier" package under the rewriter --- .../opendistroforelasticsearch/sql/query/ESActionFactory.java | 2 +- .../sql/rewriter/{ => identifier}/UnquoteIdentifierRule.java | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) rename src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/{ => identifier}/UnquoteIdentifierRule.java (95%) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/ESActionFactory.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/ESActionFactory.java index 79707421e2..58a60d0ea2 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/ESActionFactory.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/query/ESActionFactory.java @@ -41,7 +41,7 @@ import com.amazon.opendistroforelasticsearch.sql.query.multi.MultiQueryAction; import com.amazon.opendistroforelasticsearch.sql.query.multi.MultiQuerySelect; import com.amazon.opendistroforelasticsearch.sql.rewriter.RewriteRuleExecutor; -import com.amazon.opendistroforelasticsearch.sql.rewriter.UnquoteIdentifierRule; +import com.amazon.opendistroforelasticsearch.sql.rewriter.identifier.UnquoteIdentifierRule; import com.amazon.opendistroforelasticsearch.sql.rewriter.alias.TableAliasPrefixRemoveRule; import com.amazon.opendistroforelasticsearch.sql.rewriter.matchtoterm.TermFieldRewriter; import com.amazon.opendistroforelasticsearch.sql.rewriter.matchtoterm.TermFieldRewriter.TermRewriterFilter; diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/UnquoteIdentifierRule.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java similarity index 95% rename from src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/UnquoteIdentifierRule.java rename to src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java index a82ca1d43b..97e012378a 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/UnquoteIdentifierRule.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package com.amazon.opendistroforelasticsearch.sql.rewriter; +package com.amazon.opendistroforelasticsearch.sql.rewriter.identifier; import com.alibaba.druid.sql.ast.expr.SQLIdentifierExpr; import com.alibaba.druid.sql.ast.expr.SQLPropertyExpr; @@ -21,6 +21,7 @@ import com.alibaba.druid.sql.ast.statement.SQLExprTableSource; import com.alibaba.druid.sql.ast.statement.SQLSelectItem; import com.alibaba.druid.sql.dialect.mysql.visitor.MySqlASTVisitorAdapter; +import com.amazon.opendistroforelasticsearch.sql.rewriter.RewriteRule; import com.amazon.opendistroforelasticsearch.sql.utils.BackticksUnquoter; public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements RewriteRule { From eef1203a00d51b54f7e8030879446560e2233565 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Sun, 20 Oct 2019 20:07:30 -0700 Subject: [PATCH 05/27] Added unittest for the backticksUnquoter utils class --- .../unittest/utils/BackticksUnquoterTest.java | 60 +++++++++++++++++++ 1 file changed, 60 insertions(+) create mode 100644 src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java new file mode 100644 index 0000000000..2dedb2c8c8 --- /dev/null +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java @@ -0,0 +1,60 @@ +/* + * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + +package com.amazon.opendistroforelasticsearch.sql.unittest.utils; + +import com.amazon.opendistroforelasticsearch.sql.utils.BackticksUnquoter; +import org.junit.Test; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.core.IsEqual.equalTo; + +public class BackticksUnquoterTest { + + @Test + public void assertNotQuotedStringShouldKeepTheSame() { + final String originalText = "identifier"; + final String resultForUnquotingSingleField = new BackticksUnquoter().unquoteSingleField(originalText); + final String resultForUnquotingFullColumn = new BackticksUnquoter().unquoteFullColumn(originalText); + + assertThat(resultForUnquotingSingleField, equalTo(originalText)); + assertThat(resultForUnquotingFullColumn, equalTo(originalText)); + } + + @Test + public void assertStringWithOneBackTickShouldKeepTheSame() { + final String originalText = "`identifier"; + final String result1 = new BackticksUnquoter().unquoteSingleField(originalText); + final String result2 = new BackticksUnquoter().unquoteFullColumn(originalText); + + assertThat(result1, equalTo(originalText)); + assertThat(result2, equalTo(originalText)); + } + + @Test + public void assertBackticksQuotedStringShouldBeUnquoted() { + final String originalText1 = "`identifier`"; + final String originalText2 = "`identifier1`.`identifier2`"; + + final String expectedResult1 = "identifier"; + final String expectedResult2 = "identifier1.identifier2"; + + final String result1 = new BackticksUnquoter().unquoteSingleField(originalText1); + final String result2 = new BackticksUnquoter().unquoteFullColumn(originalText2); + + assertThat(expectedResult1, equalTo(result1)); + assertThat(expectedResult2, equalTo(result2)); + } +} \ No newline at end of file From f851098ea42fc23e826529cbcbdabf73dc2a5a8e Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 03:06:40 -0700 Subject: [PATCH 06/27] Added integtest for the unquote rule --- .../sql/esintgtest/QueryIT.java | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java index 603ebfe890..5611f26b73 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java @@ -1671,6 +1671,40 @@ public void fieldCollapsingTest() throws IOException { Assert.assertEquals(21, hits.length()); } + @Test + public void backticksQuotedIndexNameTest() throws Exception { + AdminClient adminClient = this.admin(); + TestUtils.createTestIndex(adminClient, "bank_two", "bank_two", null); + TestUtils.loadBulk(ESIntegTestCase.client(), "/src/test/resources/bank_two.json", "bank"); + + final String query = "SELECT lastname FROM `bank` ORDER BY age LIMIT 3"; + JSONObject response = executeQuery(query); + JSONArray hits = getHits(response); + + assertThat("bank", equalTo(((JSONObject) hits.get(0)).query("/_index"))); + } + + @Test + public void backticksQuotedFieldNamesTest() { + final String basicQuery = StringUtils.format("SELECT b.lastname FROM %s " + + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + final String queryWithQuotedAlias = StringUtils.format("SELECT `b`.lastname FROM %s" + + " AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + final String queryWithQuotedFieldName = StringUtils.format("SELECT b.`lastname` FROM %s " + + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + final String queryWithQuotedAliasAndFieldName = StringUtils.format("SELECT `b`.`lastname` FROM %s " + + "AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + final String queryWithQuotedAliasContainingSpecialChars = StringUtils.format("SELECT `b k`.lastname FROM %s " + + "AS `b k` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + + final String expectedResponse = executeQuery(basicQuery, "jdbc"); + + assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedAlias, "jdbc"))); + assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedFieldName, "jdbc"))); + assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedAliasAndFieldName, "jdbc"))); + assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedAliasContainingSpecialChars, "jdbc"))); + } + private String getScrollId(JSONObject response) { return response.getString("_scroll_id"); } From b325d442a9ded195a2f4bb801db98013b0706bd9 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 11:08:58 -0700 Subject: [PATCH 07/27] Modified the unquoter to make it reusable, and moved into StringUtils class, made corresponding changes for testing --- .../semantic/visitor/SemanticAnalyzer.java | 14 ++--- .../identifier/UnquoteIdentifierRule.java | 26 ++++---- .../sql/utils/BackticksUnquoter.java | 62 ------------------- .../sql/utils/StringUtils.java | 40 ++++++++++++ .../unittest/utils/BackticksUnquoterTest.java | 31 ++++++---- 5 files changed, 80 insertions(+), 93 deletions(-) delete mode 100644 src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/BackticksUnquoter.java diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java index e8acb5daea..3174add5b2 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java @@ -17,7 +17,7 @@ import com.amazon.opendistroforelasticsearch.sql.antlr.semantic.types.Type; import com.amazon.opendistroforelasticsearch.sql.antlr.visitor.GenericSqlParseTreeVisitor; -import com.amazon.opendistroforelasticsearch.sql.utils.BackticksUnquoter; +import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils; import java.util.List; @@ -61,20 +61,20 @@ public Type visitSelect(List itemTypes) { @Override public void visitAs(String alias, Type type) { - mappingLoader.visitAs(new BackticksUnquoter().unquoteSingleField(alias), type); - typeChecker.visitAs(new BackticksUnquoter().unquoteSingleField(alias), type); + mappingLoader.visitAs(StringUtils.unquoteSingleField(alias, "`"), type); + typeChecker.visitAs(StringUtils.unquoteSingleField(alias, "`"), type); } @Override public Type visitIndexName(String indexName) { - mappingLoader.visitIndexName(new BackticksUnquoter().unquoteSingleField(indexName)); - return typeChecker.visitIndexName(new BackticksUnquoter().unquoteSingleField(indexName)); + mappingLoader.visitIndexName(StringUtils.unquoteSingleField(indexName, "`")); + return typeChecker.visitIndexName(StringUtils.unquoteSingleField(indexName, "`")); } @Override public Type visitFieldName(String fieldName) { - mappingLoader.visitFieldName(new BackticksUnquoter().unquoteFullColumn(fieldName)); - return typeChecker.visitFieldName(new BackticksUnquoter().unquoteFullColumn(fieldName)); + mappingLoader.visitFieldName(StringUtils.unquoteFullColumn(fieldName, "`")); + return typeChecker.visitFieldName(StringUtils.unquoteFullColumn(fieldName, "`")); } @Override diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java index 97e012378a..d2689c1e02 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java @@ -22,7 +22,7 @@ import com.alibaba.druid.sql.ast.statement.SQLSelectItem; import com.alibaba.druid.sql.dialect.mysql.visitor.MySqlASTVisitorAdapter; import com.amazon.opendistroforelasticsearch.sql.rewriter.RewriteRule; -import com.amazon.opendistroforelasticsearch.sql.utils.BackticksUnquoter; +import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils; public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements RewriteRule { @@ -31,15 +31,15 @@ public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements Rew @Override public boolean visit(SQLPropertyExpr propertyExpr) { String identifier = ((SQLIdentifierExpr) propertyExpr.getOwner()).getName(); - if (identifier.startsWith("`") && identifier.endsWith("`")) { - this.identifier = new BackticksUnquoter().unquoteSingleField(identifier) + "." - + new BackticksUnquoter().unquoteSingleField(propertyExpr.getName()); - SQLSelectItem selectItem = (SQLSelectItem) propertyExpr.getParent(); - selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); - this.identifier = null; - return false; + if (!StringUtils.isQuoted(identifier, "`")) { + return true; } - return true; + this.identifier = StringUtils.unquoteSingleField(identifier, "`") + "." + + StringUtils.unquoteSingleField(propertyExpr.getName(), "`"); + SQLSelectItem selectItem = (SQLSelectItem) propertyExpr.getParent(); + selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); + this.identifier = null; + return false; } @Override @@ -47,10 +47,12 @@ public boolean visit(SQLSelectItem selectItem) { try { String identifier = ((SQLIdentifierExpr) selectItem.getExpr()).getName(); if (identifier.endsWith(".")) { - this.identifier = identifier + new BackticksUnquoter().unquoteSingleField(selectItem.getAlias()); + this.identifier = identifier + StringUtils.unquoteSingleField(selectItem.getAlias(), "`"); selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); selectItem.setAlias(null); } + } catch (ClassCastException e) { + e.printStackTrace(); } finally { return true; } @@ -70,13 +72,13 @@ public void endVisit(SQLIdentifierExpr identifierExpr) { identifierExpr.setName(identifier); identifier = null; } else { - identifierExpr.setName(new BackticksUnquoter().unquoteFullColumn(identifierExpr.getName())); + identifierExpr.setName(StringUtils.unquoteFullColumn(identifierExpr.getName(), "`")); } } @Override public void endVisit(SQLExprTableSource tableSource) { - tableSource.setAlias(new BackticksUnquoter().unquoteSingleField(tableSource.getAlias())); + tableSource.setAlias(StringUtils.unquoteSingleField(tableSource.getAlias(), "`")); } @Override diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/BackticksUnquoter.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/BackticksUnquoter.java deleted file mode 100644 index 1d921db144..0000000000 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/BackticksUnquoter.java +++ /dev/null @@ -1,62 +0,0 @@ -/* - * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. - * - * Licensed under the Apache License, Version 2.0 (the "License"). - * You may not use this file except in compliance with the License. - * A copy of the License is located at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * or in the "license" file accompanying this file. This file is distributed - * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either - * express or implied. See the License for the specific language governing - * permissions and limitations under the License. - */ - -package com.amazon.opendistroforelasticsearch.sql.utils; - -/** - * Utils Class for remove the back-ticks of identifiers - */ -public class BackticksUnquoter { - - /** - * - * @param text - * @return An unquoted string whose outer pair of back-ticks (if any) has been removed - */ - public static String unquoteSingleField(String text) { - if (text != null && text.startsWith("`") && text.endsWith("`")) { - return text.substring(1, text.length() - 1); - } - return text; - } - - /** - * - * @param text - * @return A string whose each dot-seperated field has been unquoted from back-ticks (if any) - */ - public static String unquoteFullColumn(String text) { - if (text == null) { - return null; - } - String[] strs = text.split("\\."); - for (int i = 0; i < strs.length; i++) { - String unquotedSubstr = strs[i]; - if (unquotedSubstr != null && unquotedSubstr.startsWith("`") && unquotedSubstr.endsWith("`")) { - unquotedSubstr = strs[i].substring(1, strs[i].length() - 1); - } - strs[i] = unquotedSubstr; - } - StringBuilder unquotedStrBuilder = new StringBuilder(strs[0]); - for (int i = 1; i < strs.length; i++) { - unquotedStrBuilder.append(".").append(strs[i]); - } - return unquotedStrBuilder.toString(); - } - - public BackticksUnquoter() { - } - -} diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java index 98dab9a5ef..6b4a3f9c02 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java @@ -80,6 +80,46 @@ public static int countMatches(CharSequence input, char match) { count()); } + /** + * + * @param text string + * @param quote + * @return An unquoted string whose outer pair of back-ticks (if any) has been removed + */ + public static String unquoteSingleField(String text, String quote) { + if (isQuoted(text, quote)) { + return text.substring(1, text.length() - 1); + } + return text; + } + + /** + * + * @param text + * @return A string whose each dot-seperated field has been unquoted from back-ticks (if any) + */ + public static String unquoteFullColumn(String text, String quote) { + if (text == null) { + return null; + } + String[] strs = text.split("\\."); + for (int i = 0; i < strs.length; i++) { + String unquotedSubstr = strs[i]; + if (isQuoted(unquotedSubstr, quote)) { + unquotedSubstr = strs[i].substring(1, strs[i].length() - 1); + } + strs[i] = unquotedSubstr; + } + return String.join(".", strs); + } + + public static boolean isQuoted(String text, String quote) { + if (text.length() > 1 && text.startsWith(quote) && text.endsWith(quote)) { + return true; + } + return false; + } + private StringUtils() { throw new AssertionError(getClass().getCanonicalName() + " is a utility class and must not be initialized"); } diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java index 2dedb2c8c8..19824cab29 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java @@ -15,19 +15,23 @@ package com.amazon.opendistroforelasticsearch.sql.unittest.utils; -import com.amazon.opendistroforelasticsearch.sql.utils.BackticksUnquoter; +import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils; import org.junit.Test; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.IsEqual.equalTo; +/** + * To test the functionality of {@link StringUtils#unquoteSingleField} + * and {@link StringUtils#unquoteFullColumn(String, String)} + */ public class BackticksUnquoterTest { @Test public void assertNotQuotedStringShouldKeepTheSame() { final String originalText = "identifier"; - final String resultForUnquotingSingleField = new BackticksUnquoter().unquoteSingleField(originalText); - final String resultForUnquotingFullColumn = new BackticksUnquoter().unquoteFullColumn(originalText); + final String resultForUnquotingSingleField = StringUtils.unquoteSingleField(originalText, "`"); + final String resultForUnquotingFullColumn = StringUtils.unquoteFullColumn(originalText, "`"); assertThat(resultForUnquotingSingleField, equalTo(originalText)); assertThat(resultForUnquotingFullColumn, equalTo(originalText)); @@ -35,9 +39,9 @@ public void assertNotQuotedStringShouldKeepTheSame() { @Test public void assertStringWithOneBackTickShouldKeepTheSame() { - final String originalText = "`identifier"; - final String result1 = new BackticksUnquoter().unquoteSingleField(originalText); - final String result2 = new BackticksUnquoter().unquoteFullColumn(originalText); + String originalText = "`identifier"; + String result1 = StringUtils.unquoteSingleField(originalText, "`"); + String result2 = StringUtils.unquoteFullColumn(originalText, "`"); assertThat(result1, equalTo(originalText)); assertThat(result2, equalTo(originalText)); @@ -45,16 +49,19 @@ public void assertStringWithOneBackTickShouldKeepTheSame() { @Test public void assertBackticksQuotedStringShouldBeUnquoted() { - final String originalText1 = "`identifier`"; - final String originalText2 = "`identifier1`.`identifier2`"; + String originalText1 = "`identifier`"; + String originalText2 = "`identifier1`.`identifier2`"; + String originalText3 = "`identifier1`.identifier2"; - final String expectedResult1 = "identifier"; - final String expectedResult2 = "identifier1.identifier2"; + String expectedResult1 = "identifier"; + String expectedResult2 = "identifier1.identifier2"; - final String result1 = new BackticksUnquoter().unquoteSingleField(originalText1); - final String result2 = new BackticksUnquoter().unquoteFullColumn(originalText2); + String result1 = StringUtils.unquoteSingleField(originalText1, "`"); + String result2 = StringUtils.unquoteFullColumn(originalText2, "`"); + String result3 = StringUtils.unquoteFullColumn(originalText3, "`"); assertThat(expectedResult1, equalTo(result1)); assertThat(expectedResult2, equalTo(result2)); + assertThat(expectedResult2, equalTo(result3)); } } \ No newline at end of file From 272f2c7b95e18f4163ac1f968a59444423998fab Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 11:21:48 -0700 Subject: [PATCH 08/27] Modified unittest --- .../opendistroforelasticsearch/sql/utils/StringUtils.java | 2 +- .../sql/unittest/utils/BackticksUnquoterTest.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java index 6b4a3f9c02..af3161cd47 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java @@ -114,7 +114,7 @@ public static String unquoteFullColumn(String text, String quote) { } public static boolean isQuoted(String text, String quote) { - if (text.length() > 1 && text.startsWith(quote) && text.endsWith(quote)) { + if (text != null && text.startsWith(quote) && text.endsWith(quote)) { return true; } return false; diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java index 19824cab29..f804f54e5b 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java @@ -29,9 +29,9 @@ public class BackticksUnquoterTest { @Test public void assertNotQuotedStringShouldKeepTheSame() { - final String originalText = "identifier"; - final String resultForUnquotingSingleField = StringUtils.unquoteSingleField(originalText, "`"); - final String resultForUnquotingFullColumn = StringUtils.unquoteFullColumn(originalText, "`"); + String originalText = "identifier"; + String resultForUnquotingSingleField = StringUtils.unquoteSingleField(originalText, "`"); + String resultForUnquotingFullColumn = StringUtils.unquoteFullColumn(originalText, "`"); assertThat(resultForUnquotingSingleField, equalTo(originalText)); assertThat(resultForUnquotingFullColumn, equalTo(originalText)); From 614c78efe10d1e57976749f9f54605bd1130dbe8 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 11:27:43 -0700 Subject: [PATCH 09/27] Added proper condition to modify the SQLSelectItem in visiting method --- .../sql/rewriter/identifier/UnquoteIdentifierRule.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java index d2689c1e02..d29f5ba89e 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java @@ -44,18 +44,15 @@ public boolean visit(SQLPropertyExpr propertyExpr) { @Override public boolean visit(SQLSelectItem selectItem) { - try { + if (selectItem.getExpr() instanceof SQLIdentifierExpr) { String identifier = ((SQLIdentifierExpr) selectItem.getExpr()).getName(); if (identifier.endsWith(".")) { this.identifier = identifier + StringUtils.unquoteSingleField(selectItem.getAlias(), "`"); selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); selectItem.setAlias(null); } - } catch (ClassCastException e) { - e.printStackTrace(); - } finally { - return true; } + return true; } @Override From 8a0c4c6e9742186b058f10d9bc112ec7db552359 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 11:59:36 -0700 Subject: [PATCH 10/27] Added comments to explain the major methods in the rewriter rule. --- .../identifier/UnquoteIdentifierRule.java | 53 ++++++++++++++----- 1 file changed, 39 insertions(+), 14 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java index d29f5ba89e..2fa71466ad 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java @@ -28,20 +28,17 @@ public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements Rew String identifier = null; - @Override - public boolean visit(SQLPropertyExpr propertyExpr) { - String identifier = ((SQLIdentifierExpr) propertyExpr.getOwner()).getName(); - if (!StringUtils.isQuoted(identifier, "`")) { - return true; - } - this.identifier = StringUtils.unquoteSingleField(identifier, "`") + "." - + StringUtils.unquoteSingleField(propertyExpr.getName(), "`"); - SQLSelectItem selectItem = (SQLSelectItem) propertyExpr.getParent(); - selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); - this.identifier = null; - return false; - } - + /** + * + * This method is to adjust the AST in the cases where the field is quoted, + * and the full name in the SELECT field is in the format of indexAlias.fieldName + * (e.g. SE:ECT b.`lastname` FROM bank AS b). + * + * In this case, the druid parser constructs a SQLSelectItem for the field "b.`lastname`", with SQLIdentifierExpr of + * "b." and alias of "`lastname`". + * + * This method corrects the SQLSelectItem object to have SQLIdentifier of "b.lastname" and alias of null. + */ @Override public boolean visit(SQLSelectItem selectItem) { if (selectItem.getExpr() instanceof SQLIdentifierExpr) { @@ -55,6 +52,34 @@ public boolean visit(SQLSelectItem selectItem) { return true; } + /** + * + * This method is to adjust the AST in the cases where the alias of index is quoted + * (e.g. SELECT `b`.lastname FROM bank AS `b`). + * + * In this case, the druid parser constructs a SQLPropertyExpr for the field "`b`.lastname", with owner of a + * SQLIdentifierExpr "`b`" and name of "lastname". + * + * This method prevent the visitor from visitin the SQLPropertyExpr in this case, + * and corrects AST with a SQLSelectItem object to have SQLIdentifier of "b.lastname". + * + * Used in the case where alias of index and the field name are both quoted + * (e.g. SELECT `b`.`lastname` FROM bank AS `b`). + */ + @Override + public boolean visit(SQLPropertyExpr propertyExpr) { + String fieldName = ((SQLIdentifierExpr) propertyExpr.getOwner()).getName(); + if (!StringUtils.isQuoted(fieldName, "`")) { + return true; + } + this.identifier = StringUtils.unquoteSingleField(fieldName, "`") + "." + + StringUtils.unquoteSingleField(propertyExpr.getName(), "`"); + SQLSelectItem selectItem = (SQLSelectItem) propertyExpr.getParent(); + selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); + this.identifier = null; + return false; + } + @Override public boolean visit(SQLIdentifierExpr identifierExpr) { if (identifier != null) { From 5968924ab6a93c70fa2b3cff383f0d832d2348dd Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 13:12:07 -0700 Subject: [PATCH 11/27] Moved pretty formatter test under utils test class --- .../sql/rewriter/identifier/UnquoteIdentifierRule.java | 2 +- .../sql/unittest/{ => utils}/PrettyFormatterTest.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/{ => utils}/PrettyFormatterTest.java (97%) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java index 2fa71466ad..8b33d003d9 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java @@ -26,7 +26,7 @@ public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements RewriteRule { - String identifier = null; + private String identifier = null; /** * diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/PrettyFormatterTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/PrettyFormatterTest.java similarity index 97% rename from src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/PrettyFormatterTest.java rename to src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/PrettyFormatterTest.java index 7050ea46cd..915b40fc9d 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/PrettyFormatterTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/PrettyFormatterTest.java @@ -13,7 +13,7 @@ * permissions and limitations under the License. */ -package com.amazon.opendistroforelasticsearch.sql.unittest; +package com.amazon.opendistroforelasticsearch.sql.unittest.utils; import com.amazon.opendistroforelasticsearch.sql.esintgtest.TestUtils; import com.amazon.opendistroforelasticsearch.sql.utils.JsonPrettyFormatter; From d9796c86b6639e8b303f9f90c435110d390c0262 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 13:12:28 -0700 Subject: [PATCH 12/27] Modified the unquoter IT --- .../sql/esintgtest/QueryIT.java | 5 +++-- src/test/resources/bank_for_unquote_test.json | 14 ++++++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) create mode 100644 src/test/resources/bank_for_unquote_test.json diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java index 5611f26b73..1ff40ffdb5 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java @@ -1674,8 +1674,9 @@ public void fieldCollapsingTest() throws IOException { @Test public void backticksQuotedIndexNameTest() throws Exception { AdminClient adminClient = this.admin(); - TestUtils.createTestIndex(adminClient, "bank_two", "bank_two", null); - TestUtils.loadBulk(ESIntegTestCase.client(), "/src/test/resources/bank_two.json", "bank"); + TestUtils.createTestIndex(adminClient, "bank_unquote", "bank_unquote", null); + TestUtils.loadBulk(ESIntegTestCase.client(), + "/src/test/resources/bank_for_unquote_test.json", "bank"); final String query = "SELECT lastname FROM `bank` ORDER BY age LIMIT 3"; JSONObject response = executeQuery(query); diff --git a/src/test/resources/bank_for_unquote_test.json b/src/test/resources/bank_for_unquote_test.json new file mode 100644 index 0000000000..bfce8b9ea8 --- /dev/null +++ b/src/test/resources/bank_for_unquote_test.json @@ -0,0 +1,14 @@ +{"index":{"_type": "account_unquote","_id":"1"}} +{"account_number":1,"balance":39225,"firstname":"Amber JOHnny","lastname":"Duke Willmington","age":32,"gender":"M","address":"880 Holmes Lane","employer":"Pyrami","email":"amberduke@pyrami.com","city":"Brogan","state":"IL", "male" : true, "birthdate" : "2017-10-23"} +{"index":{"_type": "account_unquote","_id":"6"}} +{"account_number":6,"balance":5686,"firstname":"Hattie","lastname":"Bond","age":36,"gender":"M","address":"671 Bristol Street","employer":"Netagy","email":"hattiebond@netagy.com","city":"Dante","state":"TN", "male" : true, "birthdate" : "2017-11-20"} +{"index":{"_type": "account_unquote","_id":"13"}} +{"account_number":13,"balance":32838,"firstname":"Nanette","lastname":"Bates","age":28,"gender":"F","address":"789 Madison Street","employer":"Quility","email":"nanettebates@quility.com","city":"Nogal","state":"VA", "male" : false, "birthdate" : "2018-06-23"} +{"index":{"_type": "account_unquote","_id":"18"}} +{"account_number":18,"balance":4180,"firstname":"Dale","lastname":"Adams","age":33,"gender":"M","address":"467 Hutchinson Court","employer":"Boink","email":"daleadams@boink.com","city":"Orick","state":"MD","male" : true, "birthdate" : "1542152000"} +{"index":{"_type": "account_unquote","_id":"20"}} +{"account_number":20,"balance":16418,"firstname":"Elinor","lastname":"Ratliff","age":36,"gender":"M","address":"282 Kings Place","employer":"Scentric","email":"elinorratliff@scentric.com","city":"Ribera","state":"WA", "male" : true, "birthdate" : "2018-06-27"} +{"index":{"_type": "account_unquote","_id":"25"}} +{"account_number":25,"balance":40540,"firstname":"Virginia","lastname":"Ayala","age":39,"gender":"F","address":"171 Putnam Avenue","employer":"Filodyne","email":"virginiaayala@filodyne.com","city":"Nicholson","state":"PA", "male" : false, "birthdate" : "2018-08-19"} +{"index":{"_type": "account_unquote","_id":"32"}} +{"account_number":32,"balance":48086,"firstname":"Dillard","lastname":"Mcpherson","age":34,"gender":"F","address":"702 Quentin Street","employer":"Quailcom","email":"dillardmcpherson@quailcom.com","city":"Veguita","state":"IN", "male" : false, "birthdate" : "2018-08-11"} From 157ed376d0747518fa457c925261ff2a0fa5456a Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 13:32:11 -0700 Subject: [PATCH 13/27] Added UT for the updated ANTLR parser --- .../SemanticAnalyzerIdentifierTest.java | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/SemanticAnalyzerIdentifierTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/SemanticAnalyzerIdentifierTest.java index f004aef50f..b8745b802a 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/SemanticAnalyzerIdentifierTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/SemanticAnalyzerIdentifierTest.java @@ -155,4 +155,21 @@ public void useConstantLiteralInSelectClauseShouldPass() { validate("SELECT TRUE FROM semantics"); } + @Test + public void queryWithBackticksQuotedIndexShouldPass() { + validate("SELECT age FROM `semantics`"); + } + + @Test + public void queryWithBackticksQuotedIndexAliasShouldPass() { + validate("SELECT `s`.age FROM semantics AS `s`"); + validate("SELECT `s t`.age FROM semantics AS `s t`"); + } + + @Test + public void queryWithBackticksQuotedFieldNameShouldPass() { + validate("SELECT `age` FROM semantics"); + validate("SELECT s.`age` FROM semantics AS s"); + validate("SELECT `s`.`age` FROM semantics AS `s`"); + } } From 76b1eb725511e017edb7870955e9b007a21f5d95 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 14:05:19 -0700 Subject: [PATCH 14/27] Used Strings.isNullOrEmpty() --- .../opendistroforelasticsearch/sql/utils/StringUtils.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java index af3161cd47..7464e98fdb 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java @@ -15,6 +15,8 @@ package com.amazon.opendistroforelasticsearch.sql.utils; +import jdk.internal.joptsimple.internal.Strings; + import java.util.Locale; /** @@ -99,7 +101,7 @@ public static String unquoteSingleField(String text, String quote) { * @return A string whose each dot-seperated field has been unquoted from back-ticks (if any) */ public static String unquoteFullColumn(String text, String quote) { - if (text == null) { + if (Strings.isNullOrEmpty(text)) { return null; } String[] strs = text.split("\\."); @@ -114,7 +116,7 @@ public static String unquoteFullColumn(String text, String quote) { } public static boolean isQuoted(String text, String quote) { - if (text != null && text.startsWith(quote) && text.endsWith(quote)) { + if (!Strings.isNullOrEmpty(text) && text.startsWith(quote) && text.endsWith(quote)) { return true; } return false; From 9ca3688ec174311a88a969f434445edacb508516 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 14:11:06 -0700 Subject: [PATCH 15/27] Reused unquoteSingleField method; modified the substring generating code --- .../opendistroforelasticsearch/sql/utils/StringUtils.java | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java index 7464e98fdb..dd2626d760 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java @@ -90,7 +90,7 @@ public static int countMatches(CharSequence input, char match) { */ public static String unquoteSingleField(String text, String quote) { if (isQuoted(text, quote)) { - return text.substring(1, text.length() - 1); + return text.substring(quote.length(), text.length() - quote.length()); } return text; } @@ -106,10 +106,7 @@ public static String unquoteFullColumn(String text, String quote) { } String[] strs = text.split("\\."); for (int i = 0; i < strs.length; i++) { - String unquotedSubstr = strs[i]; - if (isQuoted(unquotedSubstr, quote)) { - unquotedSubstr = strs[i].substring(1, strs[i].length() - 1); - } + String unquotedSubstr = unquoteSingleField(strs[i], quote); strs[i] = unquotedSubstr; } return String.join(".", strs); From 8201a8d46be0dd6c8c665ea48b147adfbcc57f12 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 14:30:07 -0700 Subject: [PATCH 16/27] Updated --- .../sql/utils/StringUtils.java | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java index dd2626d760..88ed8f60b0 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java @@ -15,7 +15,7 @@ package com.amazon.opendistroforelasticsearch.sql.utils; -import jdk.internal.joptsimple.internal.Strings; +import com.google.common.base.Strings; import java.util.Locale; @@ -101,9 +101,6 @@ public static String unquoteSingleField(String text, String quote) { * @return A string whose each dot-seperated field has been unquoted from back-ticks (if any) */ public static String unquoteFullColumn(String text, String quote) { - if (Strings.isNullOrEmpty(text)) { - return null; - } String[] strs = text.split("\\."); for (int i = 0; i < strs.length; i++) { String unquotedSubstr = unquoteSingleField(strs[i], quote); @@ -113,10 +110,7 @@ public static String unquoteFullColumn(String text, String quote) { } public static boolean isQuoted(String text, String quote) { - if (!Strings.isNullOrEmpty(text) && text.startsWith(quote) && text.endsWith(quote)) { - return true; - } - return false; + return !Strings.isNullOrEmpty(text) && text.startsWith(quote) && text.endsWith(quote); } private StringUtils() { From bc876ef6d707132c11e2cef814c674f819fdebed Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 15:23:00 -0700 Subject: [PATCH 17/27] Updated --- .../identifier/UnquoteIdentifierRule.java | 33 +++++-------------- 1 file changed, 9 insertions(+), 24 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java index 8b33d003d9..f949a0c11d 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java @@ -26,13 +26,11 @@ public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements RewriteRule { - private String identifier = null; - /** * * This method is to adjust the AST in the cases where the field is quoted, * and the full name in the SELECT field is in the format of indexAlias.fieldName - * (e.g. SE:ECT b.`lastname` FROM bank AS b). + * (e.g. SELECT b.`lastname` FROM bank AS b). * * In this case, the druid parser constructs a SQLSelectItem for the field "b.`lastname`", with SQLIdentifierExpr of * "b." and alias of "`lastname`". @@ -42,13 +40,14 @@ public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements Rew @Override public boolean visit(SQLSelectItem selectItem) { if (selectItem.getExpr() instanceof SQLIdentifierExpr) { - String identifier = ((SQLIdentifierExpr) selectItem.getExpr()).getName(); + final String identifier = ((SQLIdentifierExpr) selectItem.getExpr()).getName(); if (identifier.endsWith(".")) { - this.identifier = identifier + StringUtils.unquoteSingleField(selectItem.getAlias(), "`"); - selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); + final String correctedIdentifier = identifier + StringUtils.unquoteSingleField(selectItem.getAlias(), "`"); + selectItem.setExpr(new SQLIdentifierExpr(correctedIdentifier)); selectItem.setAlias(null); } } + selectItem.setAlias(StringUtils.unquoteSingleField(selectItem.getAlias(), "`")); return true; } @@ -68,34 +67,20 @@ public boolean visit(SQLSelectItem selectItem) { */ @Override public boolean visit(SQLPropertyExpr propertyExpr) { - String fieldName = ((SQLIdentifierExpr) propertyExpr.getOwner()).getName(); + final String fieldName = ((SQLIdentifierExpr) propertyExpr.getOwner()).getName(); if (!StringUtils.isQuoted(fieldName, "`")) { return true; } - this.identifier = StringUtils.unquoteSingleField(fieldName, "`") + "." + final String correctedIdentifier = StringUtils.unquoteSingleField(fieldName, "`") + "." + StringUtils.unquoteSingleField(propertyExpr.getName(), "`"); SQLSelectItem selectItem = (SQLSelectItem) propertyExpr.getParent(); - selectItem.setExpr(new SQLIdentifierExpr(this.identifier)); - this.identifier = null; + selectItem.setExpr(new SQLIdentifierExpr(correctedIdentifier)); return false; } - @Override - public boolean visit(SQLIdentifierExpr identifierExpr) { - if (identifier != null) { - return false; - } - return true; - } - @Override public void endVisit(SQLIdentifierExpr identifierExpr) { - if (identifier != null) { - identifierExpr.setName(identifier); - identifier = null; - } else { - identifierExpr.setName(StringUtils.unquoteFullColumn(identifierExpr.getName(), "`")); - } + identifierExpr.setName(StringUtils.unquoteFullColumn(identifierExpr.getName(), "`")); } @Override From 97eeec884a43f9b5f7f75c68b4a4717d731e41d0 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 16:19:53 -0700 Subject: [PATCH 18/27] Added IT to test the field alias in JDBC response; seperated backticks-quoted identifier ITs into index test, field test, alias test --- .../sql/esintgtest/QueryIT.java | 39 +++++++++++++++---- 1 file changed, 32 insertions(+), 7 deletions(-) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java index 1ff40ffdb5..454bb2f5d4 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java @@ -1689,23 +1689,48 @@ public void backticksQuotedIndexNameTest() throws Exception { public void backticksQuotedFieldNamesTest() { final String basicQuery = StringUtils.format("SELECT b.lastname FROM %s " + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - final String queryWithQuotedAlias = StringUtils.format("SELECT `b`.lastname FROM %s" + - " AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + final String expectedResponse = executeQuery(basicQuery, "jdbc"); + final String queryWithQuotedFieldName = StringUtils.format("SELECT b.`lastname` FROM %s " + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - final String queryWithQuotedAliasAndFieldName = StringUtils.format("SELECT `b`.`lastname` FROM %s " + - "AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - final String queryWithQuotedAliasContainingSpecialChars = StringUtils.format("SELECT `b k`.lastname FROM %s " + - "AS `b k` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedFieldName, "jdbc"))); + } + @Test + public void backticksQuotedAliasTest() { + final String basicQuery = StringUtils.format("SELECT b.lastname FROM %s " + + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); final String expectedResponse = executeQuery(basicQuery, "jdbc"); + final String queryWithQuotedAlias = StringUtils.format("SELECT `b`.lastname FROM %s" + + " AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + final String queryWithQuotedAliasAndFieldName = StringUtils.format("SELECT `b`.`lastname` FROM %s " + + "AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedAlias, "jdbc"))); - assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedFieldName, "jdbc"))); assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedAliasAndFieldName, "jdbc"))); + } + + @Test + public void backticksQuotedAliasWithSpecialCharactersTest() { + final String basicQuery = StringUtils.format("SELECT b.lastname FROM %s " + + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + final String expectedResponse = executeQuery(basicQuery, "jdbc"); + + final String queryWithQuotedAliasContainingSpecialChars = StringUtils.format("SELECT `b k`.lastname FROM %s " + + "AS `b k` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedAliasContainingSpecialChars, "jdbc"))); } + @Test + public void backticksQuotedAliasInJDBCResponseTest() { + final String query = StringUtils.format("SELECT `b`.`lastname` AS `name` FROM %s AS `b` " + + "ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + final String response = executeQuery(query, "jdbc"); + + assertTrue(response.contains("\"alias\": \"name\"")); + } + private String getScrollId(JSONObject response) { return response.getString("_scroll_id"); } From bf8445cc06555b59d9327dc5ab32dc5c9e9dfbfe Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 16:23:02 -0700 Subject: [PATCH 19/27] Updated intro of rewriter rule --- .../sql/rewriter/identifier/UnquoteIdentifierRule.java | 3 +++ .../rewriter/identifier/UnquoteIdentifierRuleTest.java | 4 ++++ 2 files changed, 7 insertions(+) create mode 100644 src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java index f949a0c11d..9b39517dd2 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java @@ -24,6 +24,9 @@ import com.amazon.opendistroforelasticsearch.sql.rewriter.RewriteRule; import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils; +/** + * Quoted Identifiers Rewriter Rule + */ public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements RewriteRule { /** diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java new file mode 100644 index 0000000000..e0aab85f48 --- /dev/null +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java @@ -0,0 +1,4 @@ +package com.amazon.opendistroforelasticsearch.sql.unittest.rewriter.identifier; + +public class UnquoteIdentifierRuleTest { +} From aa29c0f0994fdddff90420478e299dd397d545f0 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 17:04:16 -0700 Subject: [PATCH 20/27] Added unittest for UnquoteIdentifierRule --- .../identifier/UnquoteIdentifierRuleTest.java | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java index e0aab85f48..617fb8da00 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java @@ -1,4 +1,63 @@ package com.amazon.opendistroforelasticsearch.sql.unittest.rewriter.identifier; +import com.alibaba.druid.sql.SQLUtils; +import com.alibaba.druid.sql.ast.expr.SQLQueryExpr; +import com.amazon.opendistroforelasticsearch.sql.rewriter.identifier.UnquoteIdentifierRule; +import com.amazon.opendistroforelasticsearch.sql.util.SqlParserUtils; +import org.junit.Assert; +import org.junit.Test; + +/** + * Test cases for backticks quoted identifiers + */ public class UnquoteIdentifierRuleTest { + + @Test + public void queryWithBackticksQuotedIndex() { + query("SELECT lastname FROM `bank` WHERE balance > 1000 ORDER BY age" + ).shouldBeAfterRewrite("SELECT lastname FROM bank WHERE balance > 1000 ORDER BY age"); + } + + @Test + public void queryWithBackticksQuotedField() { + query("SELECT `lastname` FROM bank ORDER BY age" + ).shouldBeAfterRewrite("SELECT lastname FROM bank ORDER BY age"); + + query("SELECT b.`lastname` FROM bank AS b ORDER BY age" + ).shouldBeAfterRewrite("SELECT b.lastname FROM bank AS b ORDER BY age"); + } + + @Test + public void queryWithBackticksQuotedAlias() { + query("SELECT `b`.lastname FROM bank as `b` ORDER BY age" + ).shouldBeAfterRewrite("SELECT b.lastname FROM bank as b ORDER BY age"); + + query("SELECT `b`.`lastname` FROM `bank` as `b` ORDER BY age" + ).shouldBeAfterRewrite("SELECT b.lastname FROM bank as b ORDER BY age"); + + query("SELECT `b`.`lastname` AS `name` FROM bank as `b` ORDER BY age" + ).shouldBeAfterRewrite("SELECT b.lastname AS name FROM bank as b ORDER BY age"); + } + + private QueryAssertion query(String sql) { + return new QueryAssertion(sql); + } + + private static class QueryAssertion { + + private final UnquoteIdentifierRule rule = new UnquoteIdentifierRule(); + private final SQLQueryExpr expr; + + QueryAssertion(String sql) { + this.expr = SqlParserUtils.parse(sql); + } + + void shouldBeAfterRewrite(String expected) { + rule.rewrite(expr); + Assert.assertEquals( + SQLUtils.toMySqlString(SqlParserUtils.parse(expected)), + SQLUtils.toMySqlString(expr) + ); + } + } } From c7b42165e44c029e7adffca490e8326115cb3d22 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 18:20:01 -0700 Subject: [PATCH 21/27] Updated --- .../semantic/visitor/SemanticAnalyzer.java | 15 +++--- .../identifier/UnquoteIdentifierRule.java | 18 ++++--- .../sql/utils/StringUtils.java | 8 +++ .../sql/esintgtest/QueryIT.java | 49 +++++++++---------- .../identifier/UnquoteIdentifierRuleTest.java | 22 ++++----- .../unittest/utils/BackticksUnquoterTest.java | 34 ++++--------- 6 files changed, 69 insertions(+), 77 deletions(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java index 3174add5b2..b0b12d6a2a 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java @@ -21,6 +21,9 @@ import java.util.List; +import static com.amazon.opendistroforelasticsearch.sql.utils.StringUtils.unquoteFullColumn; +import static com.amazon.opendistroforelasticsearch.sql.utils.StringUtils.unquoteSingleField; + /** * Main visitor implementation to drive the entire semantic analysis. */ @@ -61,20 +64,20 @@ public Type visitSelect(List itemTypes) { @Override public void visitAs(String alias, Type type) { - mappingLoader.visitAs(StringUtils.unquoteSingleField(alias, "`"), type); - typeChecker.visitAs(StringUtils.unquoteSingleField(alias, "`"), type); + mappingLoader.visitAs(unquoteSingleField(alias), type); + typeChecker.visitAs(unquoteSingleField(alias), type); } @Override public Type visitIndexName(String indexName) { - mappingLoader.visitIndexName(StringUtils.unquoteSingleField(indexName, "`")); - return typeChecker.visitIndexName(StringUtils.unquoteSingleField(indexName, "`")); + mappingLoader.visitIndexName(unquoteSingleField(indexName)); + return typeChecker.visitIndexName(unquoteSingleField(indexName)); } @Override public Type visitFieldName(String fieldName) { - mappingLoader.visitFieldName(StringUtils.unquoteFullColumn(fieldName, "`")); - return typeChecker.visitFieldName(StringUtils.unquoteFullColumn(fieldName, "`")); + mappingLoader.visitFieldName(unquoteFullColumn(fieldName)); + return typeChecker.visitFieldName(unquoteFullColumn(fieldName)); } @Override diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java index 9b39517dd2..d4a454d136 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/rewriter/identifier/UnquoteIdentifierRule.java @@ -24,6 +24,9 @@ import com.amazon.opendistroforelasticsearch.sql.rewriter.RewriteRule; import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils; +import static com.amazon.opendistroforelasticsearch.sql.utils.StringUtils.unquoteFullColumn; +import static com.amazon.opendistroforelasticsearch.sql.utils.StringUtils.unquoteSingleField; + /** * Quoted Identifiers Rewriter Rule */ @@ -43,14 +46,14 @@ public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements Rew @Override public boolean visit(SQLSelectItem selectItem) { if (selectItem.getExpr() instanceof SQLIdentifierExpr) { - final String identifier = ((SQLIdentifierExpr) selectItem.getExpr()).getName(); + String identifier = ((SQLIdentifierExpr) selectItem.getExpr()).getName(); if (identifier.endsWith(".")) { - final String correctedIdentifier = identifier + StringUtils.unquoteSingleField(selectItem.getAlias(), "`"); + String correctedIdentifier = identifier + unquoteSingleField(selectItem.getAlias(), "`"); selectItem.setExpr(new SQLIdentifierExpr(correctedIdentifier)); selectItem.setAlias(null); } } - selectItem.setAlias(StringUtils.unquoteSingleField(selectItem.getAlias(), "`")); + selectItem.setAlias(unquoteSingleField(selectItem.getAlias(), "`")); return true; } @@ -70,12 +73,11 @@ public boolean visit(SQLSelectItem selectItem) { */ @Override public boolean visit(SQLPropertyExpr propertyExpr) { - final String fieldName = ((SQLIdentifierExpr) propertyExpr.getOwner()).getName(); + String fieldName = ((SQLIdentifierExpr) propertyExpr.getOwner()).getName(); if (!StringUtils.isQuoted(fieldName, "`")) { return true; } - final String correctedIdentifier = StringUtils.unquoteSingleField(fieldName, "`") + "." - + StringUtils.unquoteSingleField(propertyExpr.getName(), "`"); + String correctedIdentifier = unquoteSingleField(fieldName) + "." + unquoteSingleField(propertyExpr.getName()); SQLSelectItem selectItem = (SQLSelectItem) propertyExpr.getParent(); selectItem.setExpr(new SQLIdentifierExpr(correctedIdentifier)); return false; @@ -83,12 +85,12 @@ public boolean visit(SQLPropertyExpr propertyExpr) { @Override public void endVisit(SQLIdentifierExpr identifierExpr) { - identifierExpr.setName(StringUtils.unquoteFullColumn(identifierExpr.getName(), "`")); + identifierExpr.setName(unquoteFullColumn(identifierExpr.getName())); } @Override public void endVisit(SQLExprTableSource tableSource) { - tableSource.setAlias(StringUtils.unquoteSingleField(tableSource.getAlias(), "`")); + tableSource.setAlias(unquoteSingleField(tableSource.getAlias())); } @Override diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java index 88ed8f60b0..9a6b8c0b79 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/utils/StringUtils.java @@ -95,6 +95,10 @@ public static String unquoteSingleField(String text, String quote) { return text; } + public static String unquoteSingleField(String text) { + return unquoteSingleField(text, "`"); + } + /** * * @param text @@ -109,6 +113,10 @@ public static String unquoteFullColumn(String text, String quote) { return String.join(".", strs); } + public static String unquoteFullColumn(String text) { + return unquoteFullColumn(text, "`"); + } + public static boolean isQuoted(String text, String quote) { return !Strings.isNullOrEmpty(text) && text.startsWith(quote) && text.endsWith(quote); } diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java index 454bb2f5d4..40dee72afd 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java @@ -1678,55 +1678,50 @@ public void backticksQuotedIndexNameTest() throws Exception { TestUtils.loadBulk(ESIntegTestCase.client(), "/src/test/resources/bank_for_unquote_test.json", "bank"); - final String query = "SELECT lastname FROM `bank` ORDER BY age LIMIT 3"; - JSONObject response = executeQuery(query); - JSONArray hits = getHits(response); + String query = "SELECT lastname FROM `bank` ORDER BY age LIMIT 3"; + JSONArray hits = getHits(executeQuery(query)); assertThat("bank", equalTo(((JSONObject) hits.get(0)).query("/_index"))); } @Test public void backticksQuotedFieldNamesTest() { - final String basicQuery = StringUtils.format("SELECT b.lastname FROM %s " + - "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - final String expectedResponse = executeQuery(basicQuery, "jdbc"); + String expected = executeQuery(StringUtils.format("SELECT b.lastname FROM %s " + + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK), "jdbc"); + String quotedFieldResult = executeQuery(StringUtils.format("SELECT b.`lastname` FROM %s " + + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK), "jdbc"); - final String queryWithQuotedFieldName = StringUtils.format("SELECT b.`lastname` FROM %s " + - "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedFieldName, "jdbc"))); + assertEquals(expected, quotedFieldResult); } @Test public void backticksQuotedAliasTest() { - final String basicQuery = StringUtils.format("SELECT b.lastname FROM %s " + - "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - final String expectedResponse = executeQuery(basicQuery, "jdbc"); - - final String queryWithQuotedAlias = StringUtils.format("SELECT `b`.lastname FROM %s" + - " AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - final String queryWithQuotedAliasAndFieldName = StringUtils.format("SELECT `b`.`lastname` FROM %s " + - "AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); + String expected = executeQuery(StringUtils.format("SELECT b.lastname FROM %s " + + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK), "jdbc"); + String quotedAliasResult = executeQuery(StringUtils.format("SELECT `b`.lastname FROM %s" + + " AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK), "jdbc"); + String quotedAliasAndFieldResult = executeQuery(StringUtils.format("SELECT `b`.`lastname` FROM %s " + + "AS `b` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK), "jdbc"); - assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedAlias, "jdbc"))); - assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedAliasAndFieldName, "jdbc"))); + assertEquals(expected, quotedAliasResult); + assertEquals(expected, quotedAliasAndFieldResult); } @Test public void backticksQuotedAliasWithSpecialCharactersTest() { - final String basicQuery = StringUtils.format("SELECT b.lastname FROM %s " + - "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - final String expectedResponse = executeQuery(basicQuery, "jdbc"); + String expected = executeQuery(StringUtils.format("SELECT b.lastname FROM %s " + + "AS b ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK), "jdbc"); + String specialCharAliasResult = executeQuery(StringUtils.format("SELECT `b k`.lastname FROM %s " + + "AS `b k` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK), "jdbc"); - final String queryWithQuotedAliasContainingSpecialChars = StringUtils.format("SELECT `b k`.lastname FROM %s " + - "AS `b k` ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - assertThat(expectedResponse, equalTo(executeQuery(queryWithQuotedAliasContainingSpecialChars, "jdbc"))); + assertEquals(expected, specialCharAliasResult); } @Test public void backticksQuotedAliasInJDBCResponseTest() { - final String query = StringUtils.format("SELECT `b`.`lastname` AS `name` FROM %s AS `b` " + + String query = StringUtils.format("SELECT `b`.`lastname` AS `name` FROM %s AS `b` " + "ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK); - final String response = executeQuery(query, "jdbc"); + String response = executeQuery(query, "jdbc"); assertTrue(response.contains("\"alias\": \"name\"")); } diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java index 617fb8da00..2d505fc30d 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java @@ -13,13 +13,13 @@ public class UnquoteIdentifierRuleTest { @Test - public void queryWithBackticksQuotedIndex() { + public void queryWithQuotedIndex() { query("SELECT lastname FROM `bank` WHERE balance > 1000 ORDER BY age" ).shouldBeAfterRewrite("SELECT lastname FROM bank WHERE balance > 1000 ORDER BY age"); } @Test - public void queryWithBackticksQuotedField() { + public void queryWithQuotedField() { query("SELECT `lastname` FROM bank ORDER BY age" ).shouldBeAfterRewrite("SELECT lastname FROM bank ORDER BY age"); @@ -28,15 +28,15 @@ public void queryWithBackticksQuotedField() { } @Test - public void queryWithBackticksQuotedAlias() { - query("SELECT `b`.lastname FROM bank as `b` ORDER BY age" - ).shouldBeAfterRewrite("SELECT b.lastname FROM bank as b ORDER BY age"); + public void queryWithQuotedAlias() { + query("SELECT `b`.lastname FROM bank AS `b` ORDER BY age" + ).shouldBeAfterRewrite("SELECT b.lastname FROM bank AS b ORDER BY age"); - query("SELECT `b`.`lastname` FROM `bank` as `b` ORDER BY age" - ).shouldBeAfterRewrite("SELECT b.lastname FROM bank as b ORDER BY age"); + query("SELECT `b`.`lastname` FROM bank AS `b` ORDER BY age" + ).shouldBeAfterRewrite("SELECT b.lastname FROM bank AS b ORDER BY age"); - query("SELECT `b`.`lastname` AS `name` FROM bank as `b` ORDER BY age" - ).shouldBeAfterRewrite("SELECT b.lastname AS name FROM bank as b ORDER BY age"); + query("SELECT `b`.`lastname` AS `name` FROM bank AS `b` ORDER BY age" + ).shouldBeAfterRewrite("SELECT b.lastname AS name FROM bank AS b ORDER BY age"); } private QueryAssertion query(String sql) { @@ -45,8 +45,8 @@ private QueryAssertion query(String sql) { private static class QueryAssertion { - private final UnquoteIdentifierRule rule = new UnquoteIdentifierRule(); - private final SQLQueryExpr expr; + private UnquoteIdentifierRule rule = new UnquoteIdentifierRule(); + private SQLQueryExpr expr; QueryAssertion(String sql) { this.expr = SqlParserUtils.parse(sql); diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java index f804f54e5b..2e9bfda1a2 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/utils/BackticksUnquoterTest.java @@ -18,6 +18,8 @@ import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils; import org.junit.Test; +import static com.amazon.opendistroforelasticsearch.sql.utils.StringUtils.unquoteFullColumn; +import static com.amazon.opendistroforelasticsearch.sql.utils.StringUtils.unquoteSingleField; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.IsEqual.equalTo; @@ -29,39 +31,21 @@ public class BackticksUnquoterTest { @Test public void assertNotQuotedStringShouldKeepTheSame() { - String originalText = "identifier"; - String resultForUnquotingSingleField = StringUtils.unquoteSingleField(originalText, "`"); - String resultForUnquotingFullColumn = StringUtils.unquoteFullColumn(originalText, "`"); - - assertThat(resultForUnquotingSingleField, equalTo(originalText)); - assertThat(resultForUnquotingFullColumn, equalTo(originalText)); + assertThat(unquoteSingleField("identifier"), equalTo("identifier")); + assertThat(unquoteFullColumn("identifier"), equalTo("identifier")); } @Test public void assertStringWithOneBackTickShouldKeepTheSame() { - String originalText = "`identifier"; - String result1 = StringUtils.unquoteSingleField(originalText, "`"); - String result2 = StringUtils.unquoteFullColumn(originalText, "`"); - - assertThat(result1, equalTo(originalText)); - assertThat(result2, equalTo(originalText)); + assertThat(unquoteSingleField("`identifier"), equalTo("`identifier")); + assertThat(unquoteFullColumn("`identifier"), equalTo("`identifier")); } @Test public void assertBackticksQuotedStringShouldBeUnquoted() { - String originalText1 = "`identifier`"; - String originalText2 = "`identifier1`.`identifier2`"; - String originalText3 = "`identifier1`.identifier2"; - - String expectedResult1 = "identifier"; - String expectedResult2 = "identifier1.identifier2"; - - String result1 = StringUtils.unquoteSingleField(originalText1, "`"); - String result2 = StringUtils.unquoteFullColumn(originalText2, "`"); - String result3 = StringUtils.unquoteFullColumn(originalText3, "`"); + assertThat("identifier", equalTo(unquoteSingleField("`identifier`"))); - assertThat(expectedResult1, equalTo(result1)); - assertThat(expectedResult2, equalTo(result2)); - assertThat(expectedResult2, equalTo(result3)); + assertThat("identifier1.identifier2", equalTo(unquoteFullColumn("`identifier1`.`identifier2`"))); + assertThat("identifier1.identifier2", equalTo(unquoteFullColumn("`identifier1`.identifier2"))); } } \ No newline at end of file From 4332813beb6577e04f232448012b3a8b890ce705 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 18:22:07 -0700 Subject: [PATCH 22/27] Updated --- .../sql/antlr/semantic/visitor/SemanticAnalyzer.java | 1 - 1 file changed, 1 deletion(-) diff --git a/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java b/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java index b0b12d6a2a..dc05cae8e7 100644 --- a/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java +++ b/src/main/java/com/amazon/opendistroforelasticsearch/sql/antlr/semantic/visitor/SemanticAnalyzer.java @@ -17,7 +17,6 @@ import com.amazon.opendistroforelasticsearch.sql.antlr.semantic.types.Type; import com.amazon.opendistroforelasticsearch.sql.antlr.visitor.GenericSqlParseTreeVisitor; -import com.amazon.opendistroforelasticsearch.sql.utils.StringUtils; import java.util.List; From d66f233041a1b799a09ced4ad9af9d6f0804b60d Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 18:24:59 -0700 Subject: [PATCH 23/27] Updated --- .../identifier/UnquoteIdentifierRuleTest.java | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java index 2d505fc30d..197fffd6ce 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java @@ -1,3 +1,18 @@ +/* + * Copyright 2019 Amazon.com, Inc. or its affiliates. All Rights Reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"). + * You may not use this file except in compliance with the License. + * A copy of the License is located at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * or in the "license" file accompanying this file. This file is distributed + * on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either + * express or implied. See the License for the specific language governing + * permissions and limitations under the License. + */ + package com.amazon.opendistroforelasticsearch.sql.unittest.rewriter.identifier; import com.alibaba.druid.sql.SQLUtils; From afa4684463e1ca19d1d4172a83a6db3bc415be2f Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 18:30:11 -0700 Subject: [PATCH 24/27] Updated --- src/test/resources/bank_for_unquote_test.json | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/test/resources/bank_for_unquote_test.json b/src/test/resources/bank_for_unquote_test.json index bfce8b9ea8..73f6e41ca8 100644 --- a/src/test/resources/bank_for_unquote_test.json +++ b/src/test/resources/bank_for_unquote_test.json @@ -1,14 +1,14 @@ {"index":{"_type": "account_unquote","_id":"1"}} -{"account_number":1,"balance":39225,"firstname":"Amber JOHnny","lastname":"Duke Willmington","age":32,"gender":"M","address":"880 Holmes Lane","employer":"Pyrami","email":"amberduke@pyrami.com","city":"Brogan","state":"IL", "male" : true, "birthdate" : "2017-10-23"} +{"account_number":1,"balance":39225,"firstname":"Amber JOHnny","lastname":"Duke Willmington","age":32,"gender":"M","address":"880 Holmes Lane","employer":"Pyrami","email":"amberduke@pyrami.com","city":"Brogan","state":"IL", "male" : true"} {"index":{"_type": "account_unquote","_id":"6"}} -{"account_number":6,"balance":5686,"firstname":"Hattie","lastname":"Bond","age":36,"gender":"M","address":"671 Bristol Street","employer":"Netagy","email":"hattiebond@netagy.com","city":"Dante","state":"TN", "male" : true, "birthdate" : "2017-11-20"} +{"account_number":6,"balance":5686,"firstname":"Hattie","lastname":"Bond","age":36,"gender":"M","address":"671 Bristol Street","employer":"Netagy","email":"hattiebond@netagy.com","city":"Dante","state":"TN", "male" : true} {"index":{"_type": "account_unquote","_id":"13"}} -{"account_number":13,"balance":32838,"firstname":"Nanette","lastname":"Bates","age":28,"gender":"F","address":"789 Madison Street","employer":"Quility","email":"nanettebates@quility.com","city":"Nogal","state":"VA", "male" : false, "birthdate" : "2018-06-23"} +{"account_number":13,"balance":32838,"firstname":"Nanette","lastname":"Bates","age":28,"gender":"F","address":"789 Madison Street","employer":"Quility","email":"nanettebates@quility.com","city":"Nogal","state":"VA", "male" : false} {"index":{"_type": "account_unquote","_id":"18"}} -{"account_number":18,"balance":4180,"firstname":"Dale","lastname":"Adams","age":33,"gender":"M","address":"467 Hutchinson Court","employer":"Boink","email":"daleadams@boink.com","city":"Orick","state":"MD","male" : true, "birthdate" : "1542152000"} +{"account_number":18,"balance":4180,"firstname":"Dale","lastname":"Adams","age":33,"gender":"M","address":"467 Hutchinson Court","employer":"Boink","email":"daleadams@boink.com","city":"Orick","state":"MD","male" : true} {"index":{"_type": "account_unquote","_id":"20"}} -{"account_number":20,"balance":16418,"firstname":"Elinor","lastname":"Ratliff","age":36,"gender":"M","address":"282 Kings Place","employer":"Scentric","email":"elinorratliff@scentric.com","city":"Ribera","state":"WA", "male" : true, "birthdate" : "2018-06-27"} +{"account_number":20,"balance":16418,"firstname":"Elinor","lastname":"Ratliff","age":36,"gender":"M","address":"282 Kings Place","employer":"Scentric","email":"elinorratliff@scentric.com","city":"Ribera","state":"WA", "male" : true} {"index":{"_type": "account_unquote","_id":"25"}} -{"account_number":25,"balance":40540,"firstname":"Virginia","lastname":"Ayala","age":39,"gender":"F","address":"171 Putnam Avenue","employer":"Filodyne","email":"virginiaayala@filodyne.com","city":"Nicholson","state":"PA", "male" : false, "birthdate" : "2018-08-19"} +{"account_number":25,"balance":40540,"firstname":"Virginia","lastname":"Ayala","age":39,"gender":"F","address":"171 Putnam Avenue","employer":"Filodyne","email":"virginiaayala@filodyne.com","city":"Nicholson","state":"PA", "male" : false} {"index":{"_type": "account_unquote","_id":"32"}} -{"account_number":32,"balance":48086,"firstname":"Dillard","lastname":"Mcpherson","age":34,"gender":"F","address":"702 Quentin Street","employer":"Quailcom","email":"dillardmcpherson@quailcom.com","city":"Veguita","state":"IN", "male" : false, "birthdate" : "2018-08-11"} +{"account_number":32,"balance":48086,"firstname":"Dillard","lastname":"Mcpherson","age":34,"gender":"F","address":"702 Quentin Street","employer":"Quailcom","email":"dillardmcpherson@quailcom.com","city":"Veguita","state":"IN", "male" : false} From 44b4c1afa2bfb5d95adb40a0083da8cf542acccf Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Mon, 21 Oct 2019 18:35:10 -0700 Subject: [PATCH 25/27] Updated --- src/test/resources/bank_for_unquote_test.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/resources/bank_for_unquote_test.json b/src/test/resources/bank_for_unquote_test.json index 73f6e41ca8..6ad4c97756 100644 --- a/src/test/resources/bank_for_unquote_test.json +++ b/src/test/resources/bank_for_unquote_test.json @@ -1,5 +1,5 @@ {"index":{"_type": "account_unquote","_id":"1"}} -{"account_number":1,"balance":39225,"firstname":"Amber JOHnny","lastname":"Duke Willmington","age":32,"gender":"M","address":"880 Holmes Lane","employer":"Pyrami","email":"amberduke@pyrami.com","city":"Brogan","state":"IL", "male" : true"} +{"account_number":1,"balance":39225,"firstname":"Amber JOHnny","lastname":"Duke Willmington","age":32,"gender":"M","address":"880 Holmes Lane","employer":"Pyrami","email":"amberduke@pyrami.com","city":"Brogan","state":"IL", "male" : true} {"index":{"_type": "account_unquote","_id":"6"}} {"account_number":6,"balance":5686,"firstname":"Hattie","lastname":"Bond","age":36,"gender":"M","address":"671 Bristol Street","employer":"Netagy","email":"hattiebond@netagy.com","city":"Dante","state":"TN", "male" : true} {"index":{"_type": "account_unquote","_id":"13"}} From ba1235beadbf8b1589b92b02ecb5932bb8917e58 Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Tue, 22 Oct 2019 13:52:06 -0700 Subject: [PATCH 26/27] Added UT and IT for quoted . case --- .../sql/esintgtest/QueryIT.java | 10 +++++++--- .../identifier/UnquoteIdentifierRuleTest.java | 20 +++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java index 40dee72afd..86a01ee87a 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/esintgtest/QueryIT.java @@ -1678,10 +1678,14 @@ public void backticksQuotedIndexNameTest() throws Exception { TestUtils.loadBulk(ESIntegTestCase.client(), "/src/test/resources/bank_for_unquote_test.json", "bank"); - String query = "SELECT lastname FROM `bank` ORDER BY age LIMIT 3"; - JSONArray hits = getHits(executeQuery(query)); + JSONArray hits = getHits(executeQuery("SELECT lastname FROM `bank`")); + Object responseIndex = ((JSONObject) hits.get(0)).query("/_index"); + assertEquals("bank", responseIndex); - assertThat("bank", equalTo(((JSONObject) hits.get(0)).query("/_index"))); + assertEquals( + executeQuery("SELECT lastname FROM bank", "jdbc"), + executeQuery("SELECT `bank`.`lastname` FROM `bank`", "jdbc") + ); } @Test diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java index 197fffd6ce..501834fc6d 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java @@ -17,11 +17,15 @@ import com.alibaba.druid.sql.SQLUtils; import com.alibaba.druid.sql.ast.expr.SQLQueryExpr; +import com.amazon.opendistroforelasticsearch.sql.rewriter.RewriteRule; +import com.amazon.opendistroforelasticsearch.sql.rewriter.alias.TableAliasPrefixRemoveRule; import com.amazon.opendistroforelasticsearch.sql.rewriter.identifier.UnquoteIdentifierRule; import com.amazon.opendistroforelasticsearch.sql.util.SqlParserUtils; import org.junit.Assert; import org.junit.Test; +import java.sql.SQLFeatureNotSupportedException; + /** * Test cases for backticks quoted identifiers */ @@ -54,6 +58,13 @@ public void queryWithQuotedAlias() { ).shouldBeAfterRewrite("SELECT b.lastname AS name FROM bank AS b ORDER BY age"); } + @Test + public void selectSpecificFieldsUsingQuotedTableNamePrefix() throws SQLFeatureNotSupportedException { + query("SELECT `bank`.`lastname` FROM `bank`" + ).shouldBeAfterRewrite("SELECT bank.lastname FROM bank", + new TableAliasPrefixRemoveRule()); + } + private QueryAssertion query(String sql) { return new QueryAssertion(sql); } @@ -74,5 +85,14 @@ void shouldBeAfterRewrite(String expected) { SQLUtils.toMySqlString(expr) ); } + + void shouldBeAfterRewrite(String expected, RewriteRule extraRule) throws SQLFeatureNotSupportedException { + rule.rewrite(expr); + extraRule.rewrite(expr); + Assert.assertEquals( + SQLUtils.toMySqlString(SqlParserUtils.parse(expected)), + SQLUtils.toMySqlString(expr) + ); + } } } From 72f92debc2be69fe2ae33b2872cde3c17c6f104d Mon Sep 17 00:00:00 2001 From: chloe-zh Date: Tue, 22 Oct 2019 13:59:23 -0700 Subject: [PATCH 27/27] Added one UT case --- .../identifier/UnquoteIdentifierRuleTest.java | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java index 501834fc6d..a5c5094e70 100644 --- a/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java +++ b/src/test/java/com/amazon/opendistroforelasticsearch/sql/unittest/rewriter/identifier/UnquoteIdentifierRuleTest.java @@ -17,14 +17,11 @@ import com.alibaba.druid.sql.SQLUtils; import com.alibaba.druid.sql.ast.expr.SQLQueryExpr; -import com.amazon.opendistroforelasticsearch.sql.rewriter.RewriteRule; -import com.amazon.opendistroforelasticsearch.sql.rewriter.alias.TableAliasPrefixRemoveRule; import com.amazon.opendistroforelasticsearch.sql.rewriter.identifier.UnquoteIdentifierRule; import com.amazon.opendistroforelasticsearch.sql.util.SqlParserUtils; import org.junit.Assert; import org.junit.Test; -import java.sql.SQLFeatureNotSupportedException; /** * Test cases for backticks quoted identifiers @@ -59,10 +56,9 @@ public void queryWithQuotedAlias() { } @Test - public void selectSpecificFieldsUsingQuotedTableNamePrefix() throws SQLFeatureNotSupportedException { + public void selectSpecificFieldsUsingQuotedTableNamePrefix() { query("SELECT `bank`.`lastname` FROM `bank`" - ).shouldBeAfterRewrite("SELECT bank.lastname FROM bank", - new TableAliasPrefixRemoveRule()); + ).shouldBeAfterRewrite("SELECT bank.lastname FROM bank"); } private QueryAssertion query(String sql) { @@ -85,14 +81,5 @@ void shouldBeAfterRewrite(String expected) { SQLUtils.toMySqlString(expr) ); } - - void shouldBeAfterRewrite(String expected, RewriteRule extraRule) throws SQLFeatureNotSupportedException { - rule.rewrite(expr); - extraRule.rewrite(expr); - Assert.assertEquals( - SQLUtils.toMySqlString(SqlParserUtils.parse(expected)), - SQLUtils.toMySqlString(expr) - ); - } } }