Skip to content

Commit 9671e94

Browse files
matrivmatriv
committed
SQL: Fix issue with IN not resolving to underlying keyword field
Add resolution to the exact keyword field (if exists) for text fields. Add proper verification and error message if underlying keyword doesn't exist. Move check for field attribute in the comparison list to the `resolveType()` method of `IN`. Fixes: #38424
1 parent d862453 commit 9671e94

File tree

4 files changed

+60
-30
lines changed

4 files changed

+60
-30
lines changed

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

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,10 @@
55
*/
66
package org.elasticsearch.xpack.sql.expression.predicate.operator.comparison;
77

8+
import org.elasticsearch.xpack.sql.analysis.index.MappingException;
89
import org.elasticsearch.xpack.sql.expression.Expression;
910
import org.elasticsearch.xpack.sql.expression.Expressions;
11+
import org.elasticsearch.xpack.sql.expression.FieldAttribute;
1012
import org.elasticsearch.xpack.sql.expression.Foldables;
1113
import org.elasticsearch.xpack.sql.expression.Nullability;
1214
import org.elasticsearch.xpack.sql.expression.function.scalar.ScalarFunction;
@@ -21,6 +23,7 @@
2123
import java.util.LinkedHashSet;
2224
import java.util.List;
2325
import java.util.Objects;
26+
import java.util.Optional;
2427
import java.util.stream.Collectors;
2528

2629
import static org.elasticsearch.common.logging.LoggerMessageFormat.format;
@@ -105,6 +108,26 @@ protected Pipe makePipe() {
105108
return new InPipe(source(), this, children().stream().map(Expressions::pipe).collect(Collectors.toList()));
106109
}
107110

111+
@Override
112+
protected TypeResolution resolveType() {
113+
if (value instanceof FieldAttribute) {
114+
try {
115+
((FieldAttribute) value).exactAttribute();
116+
} catch (MappingException ex) {
117+
return new TypeResolution(format(null, "[{}] cannot operate on first argument field of data type [{}]",
118+
functionName(), value().dataType().esType));
119+
}
120+
}
121+
122+
Optional<Expression> firstNotFoldable = list.stream().filter(expression -> !expression.foldable()).findFirst();
123+
if (firstNotFoldable.isPresent()) {
124+
return new TypeResolution(format(null, "Comparisons against variables are not (currently) supported; offender [{}] in [{}]",
125+
Expressions.name(firstNotFoldable.get()),
126+
name()));
127+
}
128+
return super.resolveType();
129+
}
130+
108131
@Override
109132
public int hashCode() {
110133
return Objects.hash(value, list);

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/QueryTranslator.java‎

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,6 @@
105105
import java.util.List;
106106
import java.util.Map;
107107
import java.util.Map.Entry;
108-
import java.util.Optional;
109108
import java.util.function.Supplier;
110109

111110
import static java.util.Collections.singletonList;
@@ -708,16 +707,6 @@ static class InComparisons extends ExpressionTranslator<In> {
708707

709708
@Override
710709
protected QueryTranslation asQuery(In in, boolean onAggs) {
711-
Optional<Expression> firstNotFoldable = in.list().stream().filter(expression -> !expression.foldable()).findFirst();
712-
713-
if (firstNotFoldable.isPresent()) {
714-
throw new SqlIllegalArgumentException(
715-
"Line {}:{}: Comparisons against variables are not (currently) supported; offender [{}] in [{}]",
716-
firstNotFoldable.get().sourceLocation().getLineNumber(),
717-
firstNotFoldable.get().sourceLocation().getColumnNumber(),
718-
Expressions.name(firstNotFoldable.get()),
719-
in.name());
720-
}
721710

722711
if (in.value() instanceof NamedExpression) {
723712
NamedExpression ne = (NamedExpression) in.value();
@@ -735,7 +724,14 @@ protected QueryTranslation asQuery(In in, boolean onAggs) {
735724
else {
736725
Query q = null;
737726
if (in.value() instanceof FieldAttribute) {
738-
q = new TermsQuery(in.source(), ne.name(), in.list());
727+
FieldAttribute fa = (FieldAttribute) in.value();
728+
String name = fa.name();
729+
// equality should always be against an exact match
730+
// (which is important for strings)
731+
if (fa.isInexact()) {
732+
name = fa.exactAttribute().name();
733+
}
734+
q = new TermsQuery(in.source(), name, in.list());
739735
} else {
740736
q = new ScriptQuery(in.source(), in.asScript());
741737
}

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

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -385,23 +385,33 @@ public void testInNestedWithDifferentDataTypesFromLeftValue_SelectClause() {
385385
}
386386

387387
public void testInWithDifferentDataTypes_WhereClause() {
388-
assertEquals("1:49: expected data type [text], value provided is of type [integer]",
389-
error("SELECT * FROM test WHERE text IN ('foo', 'bar', 4)"));
388+
assertEquals("1:52: expected data type [keyword], value provided is of type [integer]",
389+
error("SELECT * FROM test WHERE keyword IN ('foo', 'bar', 4)"));
390390
}
391391

392392
public void testInNestedWithDifferentDataTypes_WhereClause() {
393-
assertEquals("1:60: expected data type [text], value provided is of type [integer]",
394-
error("SELECT * FROM test WHERE int = 1 OR text IN ('foo', 'bar', 2)"));
393+
assertEquals("1:63: expected data type [keyword], value provided is of type [integer]",
394+
error("SELECT * FROM test WHERE int = 1 OR keyword IN ('foo', 'bar', 2)"));
395395
}
396396

397397
public void testInWithDifferentDataTypesFromLeftValue_WhereClause() {
398-
assertEquals("1:35: expected data type [text], value provided is of type [integer]",
399-
error("SELECT * FROM test WHERE text IN (1, 2)"));
398+
assertEquals("1:38: expected data type [keyword], value provided is of type [integer]",
399+
error("SELECT * FROM test WHERE keyword IN (1, 2)"));
400400
}
401401

402402
public void testInNestedWithDifferentDataTypesFromLeftValue_WhereClause() {
403-
assertEquals("1:46: expected data type [text], value provided is of type [integer]",
404-
error("SELECT * FROM test WHERE int = 1 OR text IN (1, 2)"));
403+
assertEquals("1:49: expected data type [keyword], value provided is of type [integer]",
404+
error("SELECT * FROM test WHERE int = 1 OR keyword IN (1, 2)"));
405+
}
406+
407+
public void testInWithFieldInListOfValues() {
408+
assertEquals("1:26: Comparisons against variables are not (currently) supported; offender [int] in [int IN (1, int)]",
409+
error("SELECT * FROM test WHERE int IN (1, int)"));
410+
}
411+
412+
public void testInOnFieldTextWithNoKeyword() {
413+
assertEquals("1:26: [IN] cannot operate on first argument field of data type [text]",
414+
error("SELECT * FROM test WHERE text IN ('foo', 'bar')"));
405415
}
406416

407417
public void testNotSupportedAggregateOnDate() {

‎x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/planner/QueryTranslatorTests.java‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -309,8 +309,8 @@ public void testTranslateInExpression_WhereClause() {
309309
tq.asBuilder().toString().replaceAll("\\s", ""));
310310
}
311311

312-
public void testTranslateInExpression_WhereClauseAndNullHandling() {
313-
LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', null, 'lala', null, 'foo', concat('la', 'la'))");
312+
public void testTranslateInExpression_WhereClause_TextFieldWithKeyword() {
313+
LogicalPlan p = plan("SELECT * FROM test WHERE some.string IN ('foo', 'bar', 'lala', 'foo', concat('la', 'la'))");
314314
assertTrue(p instanceof Project);
315315
assertTrue(p.children().get(0) instanceof Filter);
316316
Expression condition = ((Filter) p.children().get(0)).condition();
@@ -319,21 +319,22 @@ public void testTranslateInExpression_WhereClauseAndNullHandling() {
319319
Query query = translation.query;
320320
assertTrue(query instanceof TermsQuery);
321321
TermsQuery tq = (TermsQuery) query;
322-
assertEquals("{\"terms\":{\"keyword\":[\"foo\",\"lala\"],\"boost\":1.0}}",
322+
assertEquals("{\"terms\":{\"some.string.typical\":[\"foo\",\"bar\",\"lala\"],\"boost\":1.0}}",
323323
tq.asBuilder().toString().replaceAll("\\s", ""));
324324
}
325325

326-
public void testTranslateInExpressionInvalidValues_WhereClause() {
327-
LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', 'bar', keyword)");
326+
public void testTranslateInExpression_WhereClauseAndNullHandling() {
327+
LogicalPlan p = plan("SELECT * FROM test WHERE keyword IN ('foo', null, 'lala', null, 'foo', concat('la', 'la'))");
328328
assertTrue(p instanceof Project);
329329
assertTrue(p.children().get(0) instanceof Filter);
330330
Expression condition = ((Filter) p.children().get(0)).condition();
331331
assertFalse(condition.foldable());
332-
SqlIllegalArgumentException ex = expectThrows(SqlIllegalArgumentException.class, () -> QueryTranslator.toQuery(condition, false));
333-
assertEquals(
334-
"Line 1:52: Comparisons against variables are not (currently) supported; "
335-
+ "offender [keyword] in [keyword IN ('foo', 'bar', keyword)]",
336-
ex.getMessage());
332+
QueryTranslation translation = QueryTranslator.toQuery(condition, false);
333+
Query query = translation.query;
334+
assertTrue(query instanceof TermsQuery);
335+
TermsQuery tq = (TermsQuery) query;
336+
assertEquals("{\"terms\":{\"keyword\":[\"foo\",\"lala\"],\"boost\":1.0}}",
337+
tq.asBuilder().toString().replaceAll("\\s", ""));
337338
}
338339

339340
public void testTranslateInExpression_WhereClause_Painless() {

0 commit comments

Comments
 (0)