Skip to content

Commit 22e592a

Browse files
luigidellaquilaluigidellaquila
authored
ES|QL: No plain strings in Literal (#129399)
1 parent 996c99b commit 22e592a

File tree

57 files changed

+373
-281
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

57 files changed

+373
-281
lines changed

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

Lines changed: 40 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,18 +11,24 @@
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.xpack.esql.core.QlIllegalArgumentException;
1516
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1617
import org.elasticsearch.xpack.esql.core.tree.Source;
1718
import org.elasticsearch.xpack.esql.core.type.DataType;
1819
import org.elasticsearch.xpack.esql.core.util.PlanStreamInput;
20+
import org.elasticsearch.xpack.versionfield.Version;
1921

2022
import java.io.IOException;
23+
import java.util.Collection;
2124
import java.util.List;
2225
import java.util.Objects;
2326

2427
import static org.elasticsearch.xpack.esql.core.type.DataType.CARTESIAN_POINT;
2528
import static org.elasticsearch.xpack.esql.core.type.DataType.GEO_POINT;
29+
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
30+
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
31+
import static org.elasticsearch.xpack.esql.core.type.DataType.VERSION;
2632
import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.CARTESIAN;
2733
import static org.elasticsearch.xpack.esql.core.util.SpatialCoordinateTypes.GEO;
2834

@@ -45,10 +51,25 @@ public class Literal extends LeafExpression {
4551

4652
public Literal(Source source, Object value, DataType dataType) {
4753
super(source);
54+
assert noPlainStrings(value, dataType);
4855
this.dataType = dataType;
4956
this.value = value;
5057
}
5158

59+
private boolean noPlainStrings(Object value, DataType dataType) {
60+
if (dataType == KEYWORD || dataType == TEXT || dataType == VERSION) {
61+
if (value == null) {
62+
return true;
63+
}
64+
return switch (value) {
65+
case String s -> false;
66+
case Collection<?> c -> c.stream().allMatch(x -> noPlainStrings(x, dataType));
67+
default -> true;
68+
};
69+
}
70+
return true;
71+
}
72+
5273
private static Literal readFrom(StreamInput in) throws IOException {
5374
Source source = Source.readFrom((StreamInput & PlanStreamInput) in);
5475
Object value = in.readGenericValue();
@@ -122,7 +143,21 @@ public boolean equals(Object obj) {
122143

123144
@Override
124145
public String toString() {
125-
String str = String.valueOf(value);
146+
String str;
147+
if (dataType == KEYWORD || dataType == TEXT) {
148+
str = BytesRefs.toString(value);
149+
} else if (dataType == VERSION && value instanceof BytesRef br) {
150+
str = new Version(br).toString();
151+
// TODO review how we manage IPs: https://github.com/elastic/elasticsearch/issues/129605
152+
// } else if (dataType == IP && value instanceof BytesRef ip) {
153+
// str = DocValueFormat.IP.format(ip);
154+
} else {
155+
str = String.valueOf(value);
156+
}
157+
158+
if (str == null) {
159+
str = "null";
160+
}
126161
if (str.length() > 500) {
127162
return str.substring(0, 500) + "...";
128163
}
@@ -154,6 +189,10 @@ public static Literal of(Expression source, Object value) {
154189
return new Literal(source.source(), value, source.dataType());
155190
}
156191

192+
public static Literal keyword(Source source, String literal) {
193+
return new Literal(source, BytesRefs.toBytesRef(literal), KEYWORD);
194+
}
195+
157196
/**
158197
* Not all literal values are currently supported in StreamInput/StreamOutput as generic values.
159198
* This mapper allows for addition of new and interesting values without (yet) adding to StreamInput/Output.

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1111
import org.elasticsearch.common.io.stream.StreamInput;
1212
import org.elasticsearch.common.io.stream.StreamOutput;
13+
import org.elasticsearch.common.lucene.BytesRefs;
1314
import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
1415
import org.elasticsearch.xpack.esql.core.tree.Source;
1516
import org.elasticsearch.xpack.esql.core.type.DataType;
@@ -39,7 +40,7 @@ public class MapExpression extends Expression {
3940

4041
private final Map<Expression, Expression> map;
4142

42-
private final Map<Object, Expression> keyFoldedMap;
43+
private final Map<String, Expression> keyFoldedMap;
4344

4445
public MapExpression(Source source, List<Expression> entries) {
4546
super(source, entries);
@@ -54,7 +55,7 @@ public MapExpression(Source source, List<Expression> entries) {
5455
entryExpressions.add(new EntryExpression(key.source(), key, value));
5556
map.put(key, value);
5657
if (key instanceof Literal l) {
57-
this.keyFoldedMap.put(l.value(), value);
58+
this.keyFoldedMap.put(BytesRefs.toString(l.value()), value);
5859
}
5960
}
6061
}
@@ -95,7 +96,7 @@ public Map<Expression, Expression> map() {
9596
return map;
9697
}
9798

98-
public Map<Object, Expression> keyFoldedMap() {
99+
public Map<String, Expression> keyFoldedMap() {
99100
return keyFoldedMap;
100101
}
101102

‎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 ((type == DataType.TEXT || type == DataType.KEYWORD) && value instanceof String) {
24+
value = BytesRefs.toBytesRef(value);
25+
}
2226
return new Literal(EMPTY, value, type);
2327
}
2428
}

‎x-pack/plugin/esql-core/src/test/java/org/elasticsearch/xpack/esql/core/util/TestUtils.java‎

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

88
package org.elasticsearch.xpack.esql.core.util;
99

10+
import org.elasticsearch.common.lucene.BytesRefs;
1011
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
1112
import org.elasticsearch.xpack.esql.core.expression.Literal;
1213
import org.elasticsearch.xpack.esql.core.tree.Source;
@@ -22,6 +23,8 @@
2223
import static org.elasticsearch.test.ESTestCase.randomFrom;
2324
import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY;
2425
import static org.elasticsearch.xpack.esql.core.type.DataType.INTEGER;
26+
import static org.elasticsearch.xpack.esql.core.type.DataType.KEYWORD;
27+
import static org.elasticsearch.xpack.esql.core.type.DataType.TEXT;
2528

2629
public final class TestUtils {
2730
private TestUtils() {}
@@ -39,6 +42,10 @@ public static Literal of(Source source, Object value) {
3942
if (value instanceof Literal) {
4043
return (Literal) value;
4144
}
45+
DataType type = DataType.fromJava(value);
46+
if ((type == TEXT || type == KEYWORD) && value instanceof String) {
47+
value = BytesRefs.toBytesRef(value);
48+
}
4249
return new Literal(source, value, DataType.fromJava(value));
4350
}
4451

@@ -58,12 +65,16 @@ public static FieldAttribute getFieldAttribute(String name, DataType dataType) {
5865
return new FieldAttribute(EMPTY, name, new EsField(name + "f", dataType, emptyMap(), true));
5966
}
6067

61-
/** Similar to {@link String#strip()}, but removes the WS throughout the entire string. */
68+
/**
69+
* Similar to {@link String#strip()}, but removes the WS throughout the entire string.
70+
*/
6271
public static String stripThrough(String input) {
6372
return WS_PATTERN.matcher(input).replaceAll(StringUtils.EMPTY);
6473
}
6574

66-
/** Returns the input string, but with parts of it having the letter casing changed. */
75+
/**
76+
* Returns the input string, but with parts of it having the letter casing changed.
77+
*/
6778
public static String randomCasing(String input) {
6879
StringBuilder sb = new StringBuilder(input.length());
6980
for (int i = 0, inputLen = input.length(), step = (int) Math.sqrt(inputLen); i < inputLen; i += step) {

‎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/qa/testFixtures/src/main/resources/ip.csv-spec‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -398,7 +398,7 @@ eth0 |gamma |fe80::cae2:65ff:fece:feb9
398398
lo0 |gamma |fe80::cae2:65ff:fece:feb9 |fe81::cae2:65ff:fece:feb9
399399
;
400400

401-
cdirMatchEqualsInsOrsIPs
401+
cidrMatchEqualsInsOrsIPs
402402
required_capability: combine_disjunctive_cidrmatches
403403

404404
FROM hosts

‎x-pack/plugin/esql/qa/testFixtures/src/main/resources/match-function.csv-spec‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -860,3 +860,21 @@ c: long | scalerank: long
860860
10 | 3
861861
15 | 2
862862
;
863+
864+
865+
testMatchWithReplace
866+
required_capability: match_function
867+
required_capability: no_plain_strings_in_literals
868+
from books
869+
| keep book_no, author
870+
| where match(author, REPLACE("FaulkneX", "X", "r"))
871+
| sort book_no
872+
| limit 5;
873+
874+
book_no:keyword | author:text
875+
2378 | [Carol Faulkner, Holly Byers Ochoa, Lucretia Mott]
876+
2713 | William Faulkner
877+
2847 | Colleen Faulkner
878+
2883 | William Faulkner
879+
3293 | Danny Faulkner
880+
;

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java‎

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1207,7 +1207,15 @@ public enum Cap {
12071207
/**
12081208
* Support parameters for SAMPLE command.
12091209
*/
1210-
PARAMETER_FOR_SAMPLE(Build.current().isSnapshot());
1210+
PARAMETER_FOR_SAMPLE(Build.current().isSnapshot()),
1211+
1212+
/**
1213+
* From now, Literal only accepts strings as BytesRefs.
1214+
* No java.lang.String anymore.
1215+
*
1216+
* https://github.com/elastic/elasticsearch/issues/129322
1217+
*/
1218+
NO_PLAIN_STRINGS_IN_LITERALS;
12111219

12121220
private final boolean enabled;
12131221

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
import org.elasticsearch.common.logging.HeaderWarning;
1111
import org.elasticsearch.common.logging.LoggerMessageFormat;
12+
import org.elasticsearch.common.lucene.BytesRefs;
1213
import org.elasticsearch.compute.data.Block;
1314
import org.elasticsearch.core.Strings;
1415
import org.elasticsearch.index.IndexMode;
@@ -318,7 +319,7 @@ protected LogicalPlan rule(Enrich plan, AnalyzerContext context) {
318319
// the policy does not exist
319320
return plan;
320321
}
321-
final String policyName = (String) plan.policyName().fold(FoldContext.small() /* TODO remove me */);
322+
final String policyName = BytesRefs.toString(plan.policyName().fold(FoldContext.small() /* TODO remove me */));
322323
final var resolved = context.enrichResolution().getResolvedPolicy(policyName, plan.mode());
323324
if (resolved != null) {
324325
var policy = new EnrichPolicy(resolved.matchType(), null, List.of(), resolved.matchField(), resolved.enrichFields());
@@ -401,7 +402,7 @@ private static class ResolveInference extends ParameterizedAnalyzerRule<Inferenc
401402
protected LogicalPlan rule(InferencePlan<?> plan, AnalyzerContext context) {
402403
assert plan.inferenceId().resolved() && plan.inferenceId().foldable();
403404

404-
String inferenceId = plan.inferenceId().fold(FoldContext.small()).toString();
405+
String inferenceId = BytesRefs.toString(plan.inferenceId().fold(FoldContext.small()));
405406
ResolvedInference resolvedInference = context.inferenceResolution().getResolvedInference(inferenceId);
406407

407408
if (resolvedInference != null && resolvedInference.taskType() == plan.taskType()) {
@@ -431,7 +432,7 @@ protected LogicalPlan rule(Lookup lookup, AnalyzerContext context) {
431432
// the parser passes the string wrapped in a literal
432433
Source source = lookup.source();
433434
Expression tableNameExpression = lookup.tableName();
434-
String tableName = lookup.tableName().toString();
435+
String tableName = BytesRefs.toString(tableNameExpression.fold(FoldContext.small() /* TODO remove me */));
435436
Map<String, Map<String, Column>> tables = context.configuration().tables();
436437
LocalRelation localRelation = null;
437438

@@ -1563,18 +1564,22 @@ private static boolean supportsStringImplicitCasting(DataType type) {
15631564
}
15641565

15651566
private static UnresolvedAttribute unresolvedAttribute(Expression value, String type, Exception e) {
1567+
String name = BytesRefs.toString(value.fold(FoldContext.small()) /* TODO remove me */);
15661568
String message = LoggerMessageFormat.format(
1569+
null,
15671570
"Cannot convert string [{}] to [{}], error [{}]",
1568-
value.fold(FoldContext.small() /* TODO remove me */),
1571+
name,
15691572
type,
15701573
(e instanceof ParsingException pe) ? pe.getErrorMessage() : e.getMessage()
15711574
);
1572-
return new UnresolvedAttribute(value.source(), String.valueOf(value.fold(FoldContext.small() /* TODO remove me */)), message);
1575+
return new UnresolvedAttribute(value.source(), name, message);
15731576
}
15741577

15751578
private static Expression castStringLiteralToTemporalAmount(Expression from) {
15761579
try {
1577-
TemporalAmount result = maybeParseTemporalAmount(from.fold(FoldContext.small() /* TODO remove me */).toString().strip());
1580+
TemporalAmount result = maybeParseTemporalAmount(
1581+
BytesRefs.toString(from.fold(FoldContext.small() /* TODO remove me */)).strip()
1582+
);
15781583
if (result == null) {
15791584
return from;
15801585
}

0 commit comments

Comments
 (0)