honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-03-27 18:38:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

SQL: Fix issue with IN not resolving to underlying keyword field (#38440)

Browse Source 
- Add resolution to the exact keyword field (if exists) for text fields.
- Add proper verification and error message if underlying keyword
doesn'texist.
- Move check for field attribute in the comparison list to the
`resolveType()` method of `IN`.

Fixes: #38424
This commit is contained in:
Marios Trivyzas 2019-02-06 16:19:21 +02:00
parent f8ed6c15c4
commit f96bd2ad71
No known key found for this signature in database
GPG Key ID: 8817B46B0CF36A3F
4 changed files with 59 additions and 33 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

23
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/predicate/operator/comparison/In.java
View File

@ -5,8 +5,10 @@
*/
package org.elasticsearch.xpack.sql.expression.predicate.operator.comparison;
import org.elasticsearch.xpack.sql.analysis.index.MappingException;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.expression.Expressions;
import org.elasticsearch.xpack.sql.expression.FieldAttribute;
import org.elasticsearch.xpack.sql.expression.Foldables;
import org.elasticsearch.xpack.sql.expression.Nullability;
import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction;
@ -105,6 +107,27 @@ public class In extends ScalarFunction {
return new InPipe(source(), this, children().stream().map(Expressions::pipe).collect(Collectors.toList()));
}
@Override
protected TypeResolution resolveType() {
if (value instanceof FieldAttribute) {
try {
((FieldAttribute) value).exactAttribute();
} catch (MappingException e) {
return new TypeResolution(format(null, "[{}] cannot operate on field of data type [{}]: {}",
functionName(), value().dataType().esType, e.getMessage()));
}
}
for (Expression ex : list) {
if (ex.foldable() == false) {
return new TypeResolution(format(null, "Comparisons against variables are not (currently) supported; offender [{}] in [{}]",
Expressions.name(ex),
name()));
}
}
return super.resolveType();
}
@Override
public int hashCode() {
return Objects.hash(value, list);

15
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java
View File

@ -105,7 +105,6 @@ import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.function.Supplier;
import static java.util.Collections.singletonList;
@ -708,16 +707,6 @@ final class QueryTranslator {
@Override
protected QueryTranslation asQuery(In in, boolean onAggs) {
Optional<Expression> firstNotFoldable = in.list().stream().filter(expression -> !expression.foldable()).findFirst();
if (firstNotFoldable.isPresent()) {
throw new SqlIllegalArgumentException(
"Line {}:{}: Comparisons against variables are not (currently) supported; offender [{}] in [{}]",
firstNotFoldable.get().sourceLocation().getLineNumber(),
firstNotFoldable.get().sourceLocation().getColumnNumber(),
Expressions.name(firstNotFoldable.get()),
in.name());
}
if (in.value() instanceof NamedExpression) {
NamedExpression ne = (NamedExpression) in.value();
@ -735,7 +724,9 @@ final class QueryTranslator {
else {
Query q = null;
if (in.value() instanceof FieldAttribute) {
q = new TermsQuery(in.source(), ne.name(), in.list());
FieldAttribute fa = (FieldAttribute) in.value();
// equality should always be against an exact match (which is important for strings)
q = new TermsQuery(in.source(), fa.isInexact() ? fa.exactAttribute().name() : fa.name(), in.list());
} else {
q = new ScriptQuery(in.source(), in.asScript());
}

27
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/analysis/analyzer/VerifierErrorMessagesTests.java
View File

@ -385,23 +385,34 @@ public class VerifierErrorMessagesTests extends ESTestCase {
}
public void testInWithDifferentDataTypes_WhereClause() {
assertEquals("1:49: expected data type [text], value provided is of type [integer]",
error("SELECT * FROM test WHERE text IN ('foo', 'bar', 4)"));
assertEquals("1:52: expected data type [keyword], value provided is of type [integer]",
error("SELECT * FROM test WHERE keyword IN ('foo', 'bar', 4)"));
}
public void testInNestedWithDifferentDataTypes_WhereClause() {
assertEquals("1:60: expected data type [text], value provided is of type [integer]",
error("SELECT * FROM test WHERE int = 1 OR text IN ('foo', 'bar', 2)"));
assertEquals("1:63: expected data type [keyword], value provided is of type [integer]",
error("SELECT * FROM test WHERE int = 1 OR keyword IN ('foo', 'bar', 2)"));
}
public void testInWithDifferentDataTypesFromLeftValue_WhereClause() {
assertEquals("1:35: expected data type [text], value provided is of type [integer]",
error("SELECT * FROM test WHERE text IN (1, 2)"));
assertEquals("1:38: expected data type [keyword], value provided is of type [integer]",
error("SELECT * FROM test WHERE keyword IN (1, 2)"));
}
public void testInNestedWithDifferentDataTypesFromLeftValue_WhereClause() {
assertEquals("1:46: expected data type [text], value provided is of type [integer]",
error("SELECT * FROM test WHERE int = 1 OR text IN (1, 2)"));
assertEquals("1:49: expected data type [keyword], value provided is of type [integer]",
error("SELECT * FROM test WHERE int = 1 OR keyword IN (1, 2)"));
}
public void testInWithFieldInListOfValues() {
assertEquals("1:26: Comparisons against variables are not (currently) supported; offender [int] in [int IN (1, int)]",
error("SELECT * FROM test WHERE int IN (1, int)"));
}
public void testInOnFieldTextWithNoKeyword() {
assertEquals("1:26: [IN] cannot operate on field of data type [text]: " +
"No keyword/multi-field defined exact matches for [text]; define one or use MATCH/QUERY instead",
error("SELECT * FROM test WHERE text IN ('foo', 'bar')"));
}
public void testNotSupportedAggregateOnDate() {

27
x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java
View File

@ -309,6 +309,20 @@ public class QueryTranslatorTests extends ESTestCase {
tq.asBuilder().toString().replaceAll("\\s", ""));
}
public void testTranslateInExpression_WhereClause_TextFieldWithKeyword() {
LogicalPlan p = plan("SELECT * FROM test WHERE some.string IN ('foo', 'bar', 'lala', 'foo', concat('la', 'la'))");
assertTrue(p instanceof Project);
assertTrue(p.children().get(0) instanceof Filter);
Expression condition = ((Filter) p.children().get(0)).condition();
assertFalse(condition.foldable());
QueryTranslation translation = QueryTranslator.toQuery(condition, false);
Query query = translation.query;
assertTrue(query instanceof TermsQuery);
TermsQuery tq = (TermsQuery) query;
assertEquals("{\"terms\":{\"some.string.typical\":[\"foo\",\"bar\",\"lala\"],\"boost\":1.0}}",
tq.asBuilder().toString().replaceAll("\\s", ""));
}
public void testTranslateInExpression_WhereClauseAndNullHandling() {
LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', null, 'lala', null, 'foo', concat('la', 'la'))");
assertTrue(p instanceof Project);
@ -323,19 +337,6 @@ public class QueryTranslatorTests extends ESTestCase {
tq.asBuilder().toString().replaceAll("\\s", ""));
}
public void testTranslateInExpressionInvalidValues_WhereClause() {
LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', 'bar', keyword)");
assertTrue(p instanceof Project);
assertTrue(p.children().get(0) instanceof Filter);
Expression condition = ((Filter) p.children().get(0)).condition();
assertFalse(condition.foldable());
SqlIllegalArgumentException ex = expectThrows(SqlIllegalArgumentException.class, () -> QueryTranslator.toQuery(condition, false));
assertEquals(
"Line 1:52: Comparisons against variables are not (currently) supported; "
+ "offender [keyword] in [keyword IN ('foo', 'bar', keyword)]",
ex.getMessage());
}
public void testTranslateInExpression_WhereClause_Painless() {
LogicalPlan p = plan("SELECT int FROM test WHERE POWER(int, 2) IN (10, null, 20, 30 - 10)");
assertTrue(p instanceof Project);