Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Support to parse backticks quoted identifiers #240

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
920830a
Added an unquoter utils class to extract string text from the back-ticks
Oct 21, 2019
3e3a91e
Modified the semantic analyzer to visit the index names and field nam…
Oct 21, 2019
5ef2ba8
Added an UnquoteIdentifierRule rewrite the AST that are constructed f…
Oct 21, 2019
a91696b
Moved the rule into "identifier" package under the rewriter
Oct 21, 2019
eef1203
Added unittest for the backticksUnquoter utils class
Oct 21, 2019
f851098
Added integtest for the unquote rule
Oct 21, 2019
b325d44
Modified the unquoter to make it reusable, and moved into StringUtils…
Oct 21, 2019
272f2c7
Modified unittest
Oct 21, 2019
614c78e
Added proper condition to modify the SQLSelectItem in visiting method
Oct 21, 2019
8a0c4c6
Added comments to explain the major methods in the rewriter rule.
Oct 21, 2019
5968924
Moved pretty formatter test under utils test class
Oct 21, 2019
d9796c8
Modified the unquoter IT
Oct 21, 2019
157ed37
Added UT for the updated ANTLR parser
Oct 21, 2019
76b1eb7
Used Strings.isNullOrEmpty()
Oct 21, 2019
9ca3688
Reused unquoteSingleField method; modified the substring generating code
Oct 21, 2019
8201a8d
Updated
Oct 21, 2019
bc876ef
Updated
Oct 21, 2019
97eeec8
Added IT to test the field alias in JDBC response; seperated backtick…
Oct 21, 2019
bf8445c
Updated intro of rewriter rule
Oct 21, 2019
aa29c0f
Added unittest for UnquoteIdentifierRule
Oct 22, 2019
c7b4216
Updated
Oct 22, 2019
4332813
Updated
Oct 22, 2019
d66f233
Updated
Oct 22, 2019
afa4684
Updated
Oct 22, 2019
44b4c1a
Updated
Oct 22, 2019
ba1235b
Added UT and IT for quoted <table>.<column> case
Oct 22, 2019
72f92de
Added one UT case
Oct 22, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,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.
*/
Expand Down Expand Up @@ -60,20 +63,20 @@ public Type visitSelect(List<Type> itemTypes) {

@Override
public void visitAs(String alias, Type type) {
mappingLoader.visitAs(alias, type);
typeChecker.visitAs(alias, type);
mappingLoader.visitAs(unquoteSingleField(alias), type);
typeChecker.visitAs(unquoteSingleField(alias), type);
}

@Override
public Type visitIndexName(String indexName) {
mappingLoader.visitIndexName(indexName);
return typeChecker.visitIndexName(indexName);
mappingLoader.visitIndexName(unquoteSingleField(indexName));
return typeChecker.visitIndexName(unquoteSingleField(indexName));
}

@Override
public Type visitFieldName(String fieldName) {
mappingLoader.visitFieldName(fieldName);
return typeChecker.visitFieldName(fieldName);
mappingLoader.visitFieldName(unquoteFullColumn(fieldName));
return typeChecker.visitFieldName(unquoteFullColumn(fieldName));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.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;
Expand Down Expand Up @@ -78,6 +79,7 @@ public static QueryAction create(Client client, String sql) throws SqlParseExcep

RewriteRuleExecutor<SQLQueryExpr> ruleExecutor = RewriteRuleExecutor.builder()
.withRule(new SQLExprParentSetterRule())
.withRule(new UnquoteIdentifierRule())
.withRule(new TableAliasPrefixRemoveRule())
.withRule(new SubQueryRewriteRule())
.build();
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
/*
* 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.identifier;

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.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
*/
public class UnquoteIdentifierRule extends MySqlASTVisitorAdapter implements RewriteRule<SQLQueryExpr> {

/**
*
* 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. 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`".
*
* 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) {
String identifier = ((SQLIdentifierExpr) selectItem.getExpr()).getName();
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved
if (identifier.endsWith(".")) {
String correctedIdentifier = identifier + unquoteSingleField(selectItem.getAlias(), "`");
selectItem.setExpr(new SQLIdentifierExpr(correctedIdentifier));
selectItem.setAlias(null);
}
}
selectItem.setAlias(unquoteSingleField(selectItem.getAlias(), "`"));
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) {
penghuo marked this conversation as resolved.
Show resolved Hide resolved
String fieldName = ((SQLIdentifierExpr) propertyExpr.getOwner()).getName();
if (!StringUtils.isQuoted(fieldName, "`")) {
return true;
}
String correctedIdentifier = unquoteSingleField(fieldName) + "." + unquoteSingleField(propertyExpr.getName());
SQLSelectItem selectItem = (SQLSelectItem) propertyExpr.getParent();
selectItem.setExpr(new SQLIdentifierExpr(correctedIdentifier));
return false;
}

@Override
public void endVisit(SQLIdentifierExpr identifierExpr) {
identifierExpr.setName(unquoteFullColumn(identifierExpr.getName()));
}

@Override
public void endVisit(SQLExprTableSource tableSource) {
tableSource.setAlias(unquoteSingleField(tableSource.getAlias()));
}

@Override
public boolean match(SQLQueryExpr root) {
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved
return true;
}

@Override
public void rewrite(SQLQueryExpr root) {
chloe-zh marked this conversation as resolved.
Show resolved Hide resolved
root.accept(new UnquoteIdentifierRule());
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

package com.amazon.opendistroforelasticsearch.sql.utils;

import com.google.common.base.Strings;

import java.util.Locale;

/**
Expand Down Expand Up @@ -80,6 +82,45 @@ 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(quote.length(), text.length() - quote.length());
}
return text;
}

public static String unquoteSingleField(String text) {
return unquoteSingleField(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) {
String[] strs = text.split("\\.");
for (int i = 0; i < strs.length; i++) {
String unquotedSubstr = unquoteSingleField(strs[i], quote);
strs[i] = unquotedSubstr;
}
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);
}

private StringUtils() {
throw new AssertionError(getClass().getCanonicalName() + " is a utility class and must not be initialized");
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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`");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1671,6 +1671,65 @@ public void fieldCollapsingTest() throws IOException {
Assert.assertEquals(21, hits.length());
}

@Test
public void backticksQuotedIndexNameTest() throws Exception {
AdminClient adminClient = this.admin();
TestUtils.createTestIndex(adminClient, "bank_unquote", "bank_unquote", null);
TestUtils.loadBulk(ESIntegTestCase.client(),
"/src/test/resources/bank_for_unquote_test.json", "bank");

JSONArray hits = getHits(executeQuery("SELECT lastname FROM `bank`"));
Object responseIndex = ((JSONObject) hits.get(0)).query("/_index");
assertEquals("bank", responseIndex);

assertEquals(
executeQuery("SELECT lastname FROM bank", "jdbc"),
executeQuery("SELECT `bank`.`lastname` FROM `bank`", "jdbc")
);
}

@Test
public void backticksQuotedFieldNamesTest() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we test so many queries within one method?

dai-chen marked this conversation as resolved.
Show resolved Hide resolved
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");

assertEquals(expected, quotedFieldResult);
}

@Test
public void backticksQuotedAliasTest() {
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");

assertEquals(expected, quotedAliasResult);
assertEquals(expected, quotedAliasAndFieldResult);
}

@Test
public void backticksQuotedAliasWithSpecialCharactersTest() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are there any other special characters that we should test against?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I was wondering as well if there are any other special characters that would be generated by Tableau. But I think to support the space in alias is enough based on the other chars (eg. "_" etc.) that have been supported to pass the parse so far.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SG

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");

assertEquals(expected, specialCharAliasResult);
}

@Test
public void backticksQuotedAliasInJDBCResponseTest() {
String query = StringUtils.format("SELECT `b`.`lastname` AS `name` FROM %s AS `b` " +
"ORDER BY age LIMIT 3", TestsConstants.TEST_INDEX_BANK);
String response = executeQuery(query, "jdbc");

assertTrue(response.contains("\"alias\": \"name\""));
}

private String getScrollId(JSONObject response) {
return response.getString("_scroll_id");
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
/*
* 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;
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 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 queryWithQuotedField() {
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 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` AS `name` FROM bank AS `b` ORDER BY age"
).shouldBeAfterRewrite("SELECT b.lastname AS name FROM bank AS b ORDER BY age");
}

@Test
public void selectSpecificFieldsUsingQuotedTableNamePrefix() {
query("SELECT `bank`.`lastname` FROM `bank`"
).shouldBeAfterRewrite("SELECT bank.lastname FROM bank");
}

private QueryAssertion query(String sql) {
return new QueryAssertion(sql);
}

private static class QueryAssertion {

private UnquoteIdentifierRule rule = new UnquoteIdentifierRule();
private 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)
);
}
}
}
Loading