Skip to content

Commit 48ea6e0

Browse files
luigidellaquilaluigidellaquila
committed
Only pass strings as BytesRefs
1 parent e6f4383 commit 48ea6e0

File tree

38 files changed

+214
-109
lines changed

38 files changed

+214
-109
lines changed

‎x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/analysis/AnalyzerTests.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.eql.analysis;
99

10+
import org.elasticsearch.common.lucene.BytesRefs;
1011
import org.elasticsearch.test.ESTestCase;
1112
import org.elasticsearch.xpack.eql.expression.OptionalMissingAttribute;
1213
import org.elasticsearch.xpack.eql.expression.OptionalResolvedAttribute;
@@ -75,7 +76,7 @@ public void testOptionalFieldsInsideFunction() {
7576
Concat concat = (Concat) check.left();
7677
List<Expression> arguments = new ArrayList<>(3);
7778
checkMissingOptional(concat.arguments().get(0));
78-
assertEquals(new Literal(Source.EMPTY, " ", DataTypes.KEYWORD), concat.arguments().get(1));
79+
assertEquals(new Literal(Source.EMPTY, BytesRefs.toBytesRef(" "), DataTypes.KEYWORD), concat.arguments().get(1));
7980
checkMissingOptional(concat.arguments().get(2));
8081
}
8182

‎x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/expression/Literal.java‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,12 @@
2020
import org.elasticsearch.xpack.versionfield.Version;
2121

2222
import java.io.IOException;
23+
import java.util.Collection;
2324
import java.util.List;
2425
import java.util.Objects;
2526

2627
import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_POINT;
2728
import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_POINT;
28-
import static org.elasticsearch.xpack.esql.core.type.DataType.IP;
2929
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
3030
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
3131
import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
@@ -51,19 +51,30 @@ public class Literal extends LeafExpression {
5151

5252
public Literal(Source source, Object value, DataType dataType) {
5353
super(source);
54+
assert noPlainStrings(value, dataType);
5455
this.dataType = dataType;
5556
this.value = stringsToBytesRef(dataType, value);
5657
}
5758

59+
private boolean noPlainStrings(Object value, DataType dataType) {
60+
if (dataType == KEYWORD || dataType == TEXT) {
61+
if (value instanceof String) {
62+
return false;
63+
}
64+
if (value instanceof Collection<?> c) {
65+
return c.stream().allMatch(x -> noPlainStrings(x, dataType));
66+
}
67+
}
68+
return true;
69+
}
70+
5871
@SuppressWarnings({ "unchecked", "rawtypes" })
5972
private static Object stringsToBytesRef(DataType dataType, Object value) {
60-
if (value instanceof String s) {
61-
return switch (dataType) {
62-
case IP -> s; // it's a problem for CIDR_MATCH, see CombineDisjunctions, but it's wrong!
63-
case VERSION -> new Version(s).toBytesRef();
64-
default -> BytesRefs.toBytesRef(value);
65-
};
73+
// we should do it for IPs as well, but there is a problem for CIDR_MATCH, see CombineDisjunctions
74+
if (value instanceof String s && dataType == VERSION) {
75+
return new Version(s).toBytesRef();
6676
}
77+
6778
if (value instanceof List l) {
6879
return l.stream().map(x -> stringsToBytesRef(dataType, x)).toList();
6980
}
@@ -185,7 +196,9 @@ public static Literal of(FoldContext ctx, Expression foldable) {
185196
}
186197

187198
public static Literal of(Expression source, Object value) {
188-
return new Literal(source.source(), value, source.dataType());
199+
DataType type = source.dataType();
200+
value = stringsToBytesRef(type, value);
201+
return new Literal(source.source(), value, type);
189202
}
190203

191204
/**

‎x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/LiteralTests.java‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import joptsimple.internal.Strings;
1010

11+
import org.elasticsearch.common.lucene.BytesRefs;
1112
import org.elasticsearch.test.ESTestCase;
1213
import org.elasticsearch.xpack.esql.core.InvalidArgumentException;
1314
import org.elasticsearch.xpack.esql.core.tree.AbstractNodeTestCase;
@@ -20,7 +21,6 @@
2021
import java.util.ArrayList;
2122
import java.util.Arrays;
2223
import java.util.List;
23-
import java.util.Objects;
2424
import java.util.function.Function;
2525
import java.util.function.Supplier;
2626

@@ -62,7 +62,7 @@ static class ValueAndCompatibleTypes {
6262
new ValueAndCompatibleTypes(ESTestCase::randomLong, LONG, FLOAT, DOUBLE, BOOLEAN),
6363
new ValueAndCompatibleTypes(ESTestCase::randomFloat, FLOAT, LONG, DOUBLE, BOOLEAN),
6464
new ValueAndCompatibleTypes(ESTestCase::randomDouble, DOUBLE, LONG, FLOAT, BOOLEAN),
65-
new ValueAndCompatibleTypes(() -> randomAlphaOfLength(5), KEYWORD)
65+
new ValueAndCompatibleTypes(() -> BytesRefs.toBytesRef(randomAlphaOfLength(5)), KEYWORD)
6666
);
6767

6868
public static Literal randomLiteral() {
@@ -129,14 +129,14 @@ public void testReplaceChildren() {
129129

130130
public void testToString() {
131131
assertThat(new Literal(Source.EMPTY, 1, LONG).toString(), equalTo("1"));
132-
assertThat(new Literal(Source.EMPTY, "short", KEYWORD).toString(), equalTo("short"));
132+
assertThat(new Literal(Source.EMPTY, BytesRefs.toBytesRef("short"), KEYWORD).toString(), equalTo("short"));
133133
// toString should limit it's length
134134
String tooLong = Strings.repeat('a', 510);
135-
assertThat(new Literal(Source.EMPTY, tooLong, KEYWORD).toString(), equalTo(Strings.repeat('a', 500) + "..."));
135+
assertThat(new Literal(Source.EMPTY, BytesRefs.toBytesRef(tooLong), KEYWORD).toString(), equalTo(Strings.repeat('a', 500) + "..."));
136136

137137
for (ValueAndCompatibleTypes g : GENERATORS) {
138138
Literal lit = new Literal(Source.EMPTY, g.valueSupplier.get(), randomFrom(g.validDataTypes));
139-
assertThat(lit.toString(), equalTo(Objects.toString(lit.value())));
139+
assertThat(lit.toString(), equalTo(BytesRefs.toString(lit.value())));
140140
}
141141
}
142142

‎x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/expression/function/scalar/FunctionTestUtils.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.esql.core.expression.function.scalar;
99

10+
import org.elasticsearch.common.lucene.BytesRefs;
1011
import org.elasticsearch.xpack.esql.core.expression.Literal;
1112
import org.elasticsearch.xpack.esql.core.type.DataType;
1213

@@ -15,10 +16,13 @@
1516
public final class FunctionTestUtils {
1617

1718
public static Literal l(Object value) {
18-
return new Literal(EMPTY, value, DataType.fromJava(value));
19+
return l(value, DataType.fromJava(value));
1920
}
2021

2122
public static Literal l(Object value, DataType type) {
23+
if (value instanceof String && (type == DataType.TEXT || type == DataType.KEYWORD)) {
24+
value = BytesRefs.toBytesRef(value);
25+
}
2226
return new Literal(EMPTY, value, type);
2327
}
2428
}

‎x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import org.elasticsearch.common.breaker.CircuitBreaker;
2222
import org.elasticsearch.common.breaker.NoopCircuitBreaker;
2323
import org.elasticsearch.common.bytes.BytesReference;
24+
import org.elasticsearch.common.lucene.BytesRefs;
2425
import org.elasticsearch.common.regex.Regex;
2526
import org.elasticsearch.common.settings.Settings;
2627
import org.elasticsearch.common.util.BigArrays;
@@ -227,7 +228,11 @@ public static Literal of(Source source, Object value) {
227228
if (value instanceof Literal) {
228229
return (Literal) value;
229230
}
230-
return new Literal(source, value, DataType.fromJava(value));
231+
var dataType = DataType.fromJava(value);
232+
if (value instanceof String) {
233+
value = BytesRefs.toBytesRef(value);
234+
}
235+
return new Literal(source, value, dataType);
231236
}
232237

233238
public static ReferenceAttribute referenceAttribute(String name, DataType type) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Count.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1111
import org.elasticsearch.common.io.stream.StreamInput;
12+
import org.elasticsearch.common.lucene.BytesRefs;
1213
import org.elasticsearch.compute.aggregation.AggregatorFunctionSupplier;
1314
import org.elasticsearch.compute.aggregation.CountAggregatorFunction;
1415
import org.elasticsearch.compute.data.AggregateMetricDoubleBlockBuilder;
@@ -176,7 +177,7 @@ public Expression surrogate() {
176177
return new Mul(
177178
s,
178179
new Coalesce(s, new MvCount(s, field), List.of(new Literal(s, 0, DataType.INTEGER))),
179-
new Count(s, new Literal(s, StringUtils.WILDCARD, DataType.KEYWORD))
180+
new Count(s, new Literal(s, BytesRefs.toBytesRef(StringUtils.WILDCARD), DataType.KEYWORD))
180181
);
181182
}
182183

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/aggregate/Sum.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1010
import org.elasticsearch.common.io.stream.StreamInput;
11+
import org.elasticsearch.common.lucene.BytesRefs;
1112
import org.elasticsearch.compute.aggregation.AggregatorFunctionSupplier;
1213
import org.elasticsearch.compute.aggregation.SumDoubleAggregatorFunctionSupplier;
1314
import org.elasticsearch.compute.aggregation.SumIntAggregatorFunctionSupplier;
@@ -157,7 +158,7 @@ public Expression surrogate() {
157158

158159
// SUM(const) is equivalent to MV_SUM(const)*COUNT(*).
159160
return field.foldable()
160-
? new Mul(s, new MvSum(s, field), new Count(s, new Literal(s, StringUtils.WILDCARD, DataType.KEYWORD)))
161+
? new Mul(s, new MvSum(s, field), new Count(s, new Literal(s, BytesRefs.toBytesRef(StringUtils.WILDCARD), DataType.KEYWORD)))
161162
: null;
162163
}
163164

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/Space.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1212
import org.elasticsearch.common.io.stream.StreamInput;
1313
import org.elasticsearch.common.io.stream.StreamOutput;
14+
import org.elasticsearch.common.lucene.BytesRefs;
1415
import org.elasticsearch.compute.ann.Evaluator;
1516
import org.elasticsearch.compute.ann.Fixed;
1617
import org.elasticsearch.compute.operator.BreakingBytesRefBuilder;
@@ -116,7 +117,7 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
116117
Object folded = field.fold(toEvaluator.foldCtx());
117118
if (folded instanceof Integer num) {
118119
checkNumber(num);
119-
return toEvaluator.apply(new Literal(source(), " ".repeat(num), KEYWORD));
120+
return toEvaluator.apply(new Literal(source(), BytesRefs.toBytesRef(" ".repeat(num)), KEYWORD));
120121
}
121122
}
122123

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceRegexMatch.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
package org.elasticsearch.xpack.esql.optimizer.rules.logical;
99

10+
import org.elasticsearch.common.lucene.BytesRefs;
1011
import org.elasticsearch.xpack.esql.core.expression.Expression;
1112
import org.elasticsearch.xpack.esql.core.expression.Literal;
1213
import org.elasticsearch.xpack.esql.core.expression.predicate.regex.RegexMatch;
@@ -43,7 +44,7 @@ public Expression rule(RegexMatch<?> regexMatch, LogicalOptimizerContext ctx) {
4344
} else {
4445
String match = pattern.exactMatch();
4546
if (match != null) {
46-
Literal literal = new Literal(regexMatch.source(), match, DataType.KEYWORD);
47+
Literal literal = new Literal(regexMatch.source(), BytesRefs.toBytesRef(match), DataType.KEYWORD);
4748
e = regexToEquals(regexMatch, literal);
4849
}
4950
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
import static org.elasticsearch.xpack.esql.core.type.DataType.DATE_PERIOD;
8383
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
8484
import static org.elasticsearch.xpack.esql.core.type.DataType.NULL;
85+
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
8586
import static org.elasticsearch.xpack.esql.core.type.DataType.TIME_DURATION;
8687
import static org.elasticsearch.xpack.esql.core.util.NumericUtils.asLongUnsigned;
8788
import static org.elasticsearch.xpack.esql.core.util.NumericUtils.unsignedLongAsNumber;
@@ -659,7 +660,7 @@ public MapExpression visitMapExpression(EsqlBaseParser.MapExpressionContext ctx)
659660
if (l.dataType() == NULL) {
660661
throw new ParsingException(source(ctx), "Invalid named function argument [{}], NULL is not supported", entryText);
661662
}
662-
namedArgs.add(new Literal(source(stringCtx), key, KEYWORD));
663+
namedArgs.add(new Literal(source(stringCtx), BytesRefs.toBytesRef(key), KEYWORD));
663664
namedArgs.add(l);
664665
names.add(key);
665666
} else {
@@ -941,6 +942,9 @@ private Expression visitParam(EsqlBaseParser.ParameterContext ctx, QueryParam pa
941942
return new UnresolvedAttribute(source, value.toString());
942943
}
943944
}
945+
if ((type == KEYWORD || type == TEXT) && value instanceof String) {
946+
value = BytesRefs.toBytesRef(value);
947+
}
944948
return new Literal(source, value, type);
945949
}
946950

0 commit comments

Comments
 (0)