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

Commit

Permalink
Bug fix, ignore the term query rewrite if there is no index found (#425)
Browse files Browse the repository at this point in the history
* Bug fix, ignore the term query rewrite if there is no index found

* move IsEqualIgnoreCaseAndWhiteSpace to MatcherUtils
  • Loading branch information
penghuo authored Apr 12, 2020
1 parent 45fa29c commit 2bfe326
Show file tree
Hide file tree
Showing 4 changed files with 132 additions and 9 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -105,17 +105,17 @@ public static QueryAction create(Client client, QueryActionRequest request)
sqlExpr.accept(new NestedFieldRewriter());

if (isMulti(sqlExpr)) {
sqlExpr.accept(new TermFieldRewriter(client, TermRewriterFilter.MULTI_QUERY));
sqlExpr.accept(new TermFieldRewriter(TermRewriterFilter.MULTI_QUERY));
MultiQuerySelect multiSelect =
new SqlParser().parseMultiSelect((SQLUnionQuery) sqlExpr.getSubQuery().getQuery());
return new MultiQueryAction(client, multiSelect);
} else if (isJoin(sqlExpr, sql)) {
new JoinRewriteRule(LocalClusterState.state()).rewrite(sqlExpr);
sqlExpr.accept(new TermFieldRewriter(client, TermRewriterFilter.JOIN));
sqlExpr.accept(new TermFieldRewriter(TermRewriterFilter.JOIN));
JoinSelect joinSelect = new SqlParser().parseJoinSelect(sqlExpr);
return ESJoinQueryActionFactory.createJoinAction(client, joinSelect);
} else {
sqlExpr.accept(new TermFieldRewriter(client));
sqlExpr.accept(new TermFieldRewriter());
// migrate aggregation to query planner framework.
if (shouldMigrateToQueryPlan(sqlExpr, request.getFormat())) {
return new QueryPlanQueryAction(new QueryPlanRequestBuilder(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
import com.amazon.opendistroforelasticsearch.sql.esdomain.LocalClusterState;
import com.amazon.opendistroforelasticsearch.sql.esdomain.mapping.FieldMappings;
import com.amazon.opendistroforelasticsearch.sql.esdomain.mapping.IndexMappings;
import org.elasticsearch.client.Client;
import org.json.JSONObject;

import java.util.ArrayDeque;
Expand All @@ -56,16 +55,13 @@
public class TermFieldRewriter extends MySqlASTVisitorAdapter {

private Deque<TermFieldScope> environment = new ArrayDeque<>();
private Client client;
private TermRewriterFilter filterType;

public TermFieldRewriter(Client client) {
this.client = client;
public TermFieldRewriter() {
this.filterType = TermRewriterFilter.COMMA;
}

public TermFieldRewriter(Client client, TermRewriterFilter filterType) {
this.client = client;
public TermFieldRewriter(TermRewriterFilter filterType) {
this.filterType = filterType;
}

Expand All @@ -78,6 +74,10 @@ public boolean visit(MySqlSelectQueryBlock query) {

Map<String, String> indexToType = new HashMap<>();
collect(query.getFrom(), indexToType, curScope().getAliases());
if (indexToType.isEmpty()) {
// no index found for current scope, continue.
return true;
}
curScope().setMapper(getMappings(indexToType));

if (this.filterType == TermRewriterFilter.COMMA || this.filterType == TermRewriterFilter.MULTI_QUERY) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
/*
* 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.term;


import com.alibaba.druid.sql.SQLUtils;
import com.alibaba.druid.sql.ast.expr.SQLQueryExpr;
import com.amazon.opendistroforelasticsearch.sql.esdomain.LocalClusterState;
import com.amazon.opendistroforelasticsearch.sql.rewriter.matchtoterm.TermFieldRewriter;
import com.amazon.opendistroforelasticsearch.sql.util.MatcherUtils;
import com.amazon.opendistroforelasticsearch.sql.util.SqlParserUtils;
import com.google.common.base.Charsets;
import com.google.common.io.Resources;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.ExpectedException;

import java.io.IOException;
import java.net.URL;

import static com.amazon.opendistroforelasticsearch.sql.util.CheckScriptContents.mockLocalClusterState;
import static org.hamcrest.MatcherAssert.assertThat;

public class TermFieldRewriterTest {
private static final String TEST_MAPPING_FILE = "mappings/semantics.json";

@Rule
public ExpectedException exception = ExpectedException.none();

@Before
public void setup() throws IOException {
URL url = Resources.getResource(TEST_MAPPING_FILE);
String mappings = Resources.toString(url, Charsets.UTF_8);
LocalClusterState.state(null);
mockLocalClusterState(mappings);
}

@Test
public void testFromSubqueryShouldPass() {
String sql = "SELECT t.age as a FROM (SELECT age FROM semantics WHERE employer = 'david') t";
String expected = "SELECT t.age as a FROM (SELECT age FROM semantics WHERE employer.keyword = 'david') t";

assertThat(rewriteTerm(sql),
MatcherUtils.IsEqualIgnoreCaseAndWhiteSpace.equalToIgnoreCaseAndWhiteSpace(expected));
}

@Test
public void testFromSubqueryWithoutTermShouldPass() {
String sql = "SELECT t.age as a FROM (SELECT age FROM semantics WHERE age = 10) t";
String expected = sql;

assertThat(rewriteTerm(sql),
MatcherUtils.IsEqualIgnoreCaseAndWhiteSpace.equalToIgnoreCaseAndWhiteSpace(expected));
}

private String rewriteTerm(String sql) {
SQLQueryExpr sqlQueryExpr = SqlParserUtils.parse(sql);
sqlQueryExpr.accept(new TermFieldRewriter());
return SQLUtils.toMySqlString(sqlQueryExpr)
.replaceAll("[\\n\\t]+", " ")
.replaceAll("^\\(", " ")
.replaceAll("\\)$", " ")
.trim();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -195,4 +195,48 @@ protected boolean matchesSafely(JSONArray array) {
}
};
}


/**
* Tests if a string is equal to another string, ignore the case and whitespace.
*/
public static class IsEqualIgnoreCaseAndWhiteSpace extends TypeSafeMatcher<String> {
private final String string;

public IsEqualIgnoreCaseAndWhiteSpace(String string) {
if (string == null) {
throw new IllegalArgumentException("Non-null value required");
}
this.string = string;
}

@Override
public boolean matchesSafely(String item) {
return ignoreCase(ignoreSpaces(string)).equals(ignoreCase(ignoreSpaces(item)));
}

@Override
public void describeMismatchSafely(String item, Description mismatchDescription) {
mismatchDescription.appendText("was ").appendValue(item);
}

@Override
public void describeTo(Description description) {
description.appendText("a string equal to ")
.appendValue(string)
.appendText(" ignore case and white space");
}

public String ignoreSpaces(String toBeStripped) {
return toBeStripped.replaceAll("\\s+", "").trim();
}

public String ignoreCase(String toBeLower) {
return toBeLower.toLowerCase();
}

public static Matcher<String> equalToIgnoreCaseAndWhiteSpace(String expectedString) {
return new IsEqualIgnoreCaseAndWhiteSpace(expectedString);
}
}
}

0 comments on commit 2bfe326

Please sign in to comment.