Skip to content

Commit aff0fac

Browse files
Address code review feedback part 1
1 parent eb1deb9 commit aff0fac

File tree

6 files changed

+39
-30
lines changed

6 files changed

+39
-30
lines changed

server/src/main/java/org/elasticsearch/index/mapper/MappedFieldType.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -331,8 +331,8 @@ public final Query wildcardQuery(String value, @Nullable MultiTermQuery.RewriteM
331331
}
332332

333333
/**
334-
* Similar to wildcardQuery, except that we change the behavior for
335-
* this method for IndexFieldMapper
334+
* Similar to wildcardQuery, except that we change the behavior for ESQL
335+
* to behave like a string LIKE query, where the value is matched as a string
336336
*/
337337
public Query wildcardLikeQuery(
338338
String value,

server/src/main/java/org/elasticsearch/index/query/WildcardQueryBuilder.java

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ public class WildcardQueryBuilder extends AbstractQueryBuilder<WildcardQueryBuil
5656
private static final ParseField CASE_INSENSITIVE_FIELD = new ParseField("case_insensitive");
5757
private boolean caseInsensitive = DEFAULT_CASE_INSENSITIVITY;
5858

59-
private boolean isForESQL = false;
59+
// forces a string like match instead of a wildcard match
60+
private boolean forceStringMatch = false;
6061

6162
/**
6263
* Implements the wildcard search query. Supported wildcards are {@code *}, which
@@ -82,7 +83,7 @@ public WildcardQueryBuilder(String fieldName, String value) {
8283

8384
public WildcardQueryBuilder(String fieldName, String value, boolean isForESQL) {
8485
this(fieldName, value);
85-
this.isForESQL = isForESQL;
86+
this.forceStringMatch = isForESQL;
8687
}
8788

8889
/**
@@ -95,9 +96,9 @@ public WildcardQueryBuilder(StreamInput in) throws IOException {
9596
rewrite = in.readOptionalString();
9697
caseInsensitive = in.readBoolean();
9798
if (in.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIXED_INDEX_LIKE)) {
98-
isForESQL = in.readBoolean();
99+
forceStringMatch = in.readBoolean();
99100
} else {
100-
isForESQL = false;
101+
forceStringMatch = false;
101102
}
102103
}
103104

@@ -108,7 +109,7 @@ protected void doWriteTo(StreamOutput out) throws IOException {
108109
out.writeOptionalString(rewrite);
109110
out.writeBoolean(caseInsensitive);
110111
if (out.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIXED_INDEX_LIKE)) {
111-
out.writeBoolean(isForESQL);
112+
out.writeBoolean(forceStringMatch);
112113
}
113114
}
114115

@@ -234,7 +235,7 @@ private QueryBuilder maybeRewriteBasedOnConstantFields(@Nullable MappedFieldType
234235
// fields we also have the guarantee that it doesn't perform I/O, which is important
235236
// since rewrites might happen on a network thread.
236237
Query query;
237-
if (isForESQL) {
238+
if (forceStringMatch) {
238239
query = constantFieldType.wildcardLikeQuery(value, caseInsensitive, context); // the rewrite method doesn't matter
239240
} else {
240241
query = constantFieldType.wildcardQuery(value, caseInsensitive, context); // the rewrite method doesn't matter

x-pack/plugin/esql/qa/server/multi-clusters/src/javaRestTest/java/org/elasticsearch/xpack/esql/ccq/MultiClustersIT.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -293,7 +293,7 @@ private void assertClusterDetailsMap(Map<String, Object> result, boolean remoteO
293293
assertThat(remoteClusterShards.keySet(), equalTo(Set.of("total", "successful", "skipped", "failed")));
294294
assertThat((Integer) remoteClusterShards.get("total"), greaterThanOrEqualTo(0));
295295
assertThat((Integer) remoteClusterShards.get("successful"), equalTo((Integer) remoteClusterShards.get("total")));
296-
// assertThat((Integer) remoteClusterShards.get("skipped"), equalTo(0));
296+
assertThat((Integer) remoteClusterShards.get("skipped"), greaterThanOrEqualTo(0));
297297
assertThat((Integer) remoteClusterShards.get("failed"), equalTo(0));
298298

299299
if (remoteOnly == false) {
@@ -309,7 +309,7 @@ private void assertClusterDetailsMap(Map<String, Object> result, boolean remoteO
309309
assertThat(localClusterShards.keySet(), equalTo(Set.of("total", "successful", "skipped", "failed")));
310310
assertThat((Integer) localClusterShards.get("total"), greaterThanOrEqualTo(0));
311311
assertThat((Integer) localClusterShards.get("successful"), equalTo((Integer) localClusterShards.get("total")));
312-
// assertThat((Integer) localClusterShards.get("skipped"), equalTo(0));
312+
assertThat((Integer) localClusterShards.get("skipped"), greaterThanOrEqualTo(0));
313313
assertThat((Integer) localClusterShards.get("failed"), equalTo(0));
314314
}
315315
}

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/capabilities/TranslationAware.java

Lines changed: 16 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@
77

88
package org.elasticsearch.xpack.esql.capabilities;
99

10-
import org.apache.lucene.util.automaton.Automaton;
10+
import org.apache.lucene.search.MultiTermQuery.RewriteMethod;
1111
import org.elasticsearch.compute.lucene.LuceneTopNSourceOperator;
1212
import org.elasticsearch.compute.operator.FilterOperator;
13+
import org.elasticsearch.index.mapper.MappedFieldType;
14+
import org.elasticsearch.index.query.SearchExecutionContext;
1315
import org.elasticsearch.xpack.esql.core.expression.Expression;
1416
import org.elasticsearch.xpack.esql.core.querydsl.query.Query;
1517
import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
@@ -49,19 +51,21 @@ static TranslationAware.Translatable translatable(Expression exp, LucenePushdown
4951
Query asQuery(LucenePushdownPredicates pushdownPredicates, TranslatorHandler handler);
5052

5153
/**
52-
* Translates the implementing expression into a Lucene {@link Automaton}.
54+
* Translates this expression into a Lucene {@link org.apache.lucene.search.Query}.
55+
* <p>
56+
* Implementations should use the provided field type, rewrite method, and search execution context
57+
* to construct an appropriate Lucene query for this expression.
58+
* By default, this method throws {@link UnsupportedOperationException}; override it in subclasses
59+
* that support Lucene query translation.
60+
* </p>
5361
*/
54-
default Automaton asLuceneQuery() {
62+
default org.apache.lucene.search.Query asLuceneQuery(
63+
MappedFieldType fieldType,
64+
RewriteMethod constantScoreRewrite,
65+
SearchExecutionContext context
66+
) {
5567
throw new UnsupportedOperationException("asLuceneQuery is not implemented for " + getClass().getName());
56-
};
57-
58-
/**
59-
* Returns a description of the Lucene query that this expression translates to.
60-
* This is used for debugging and logging purposes.
61-
*/
62-
default String getLuceneQueryDescription() {
63-
throw new UnsupportedOperationException("getLuceneQueryDescription is not implemented for " + getClass().getName());
64-
};
68+
}
6569

6670
/**
6771
* Subinterface for expressions that can only process single values (and null out on MVs).

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,12 +7,15 @@
77

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

10+
import org.apache.lucene.search.MultiTermQuery.RewriteMethod;
1011
import org.apache.lucene.util.automaton.Automaton;
1112
import org.elasticsearch.TransportVersions;
1213
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
1314
import org.elasticsearch.common.io.stream.StreamInput;
1415
import org.elasticsearch.common.io.stream.StreamOutput;
1516
import org.elasticsearch.core.Nullable;
17+
import org.elasticsearch.index.mapper.MappedFieldType;
18+
import org.elasticsearch.index.query.SearchExecutionContext;
1619
import org.elasticsearch.xpack.esql.core.expression.Expression;
1720
import org.elasticsearch.xpack.esql.core.expression.FieldAttribute;
1821
import org.elasticsearch.xpack.esql.core.expression.predicate.regex.WildcardPattern;
@@ -150,12 +153,16 @@ public Query asQuery(LucenePushdownPredicates pushdownPredicates, TranslatorHand
150153
}
151154

152155
@Override
153-
public Automaton asLuceneQuery() {
154-
return pattern().createAutomaton(caseInsensitive());
156+
public org.apache.lucene.search.Query asLuceneQuery(
157+
MappedFieldType fieldType,
158+
RewriteMethod constantScoreRewrite,
159+
SearchExecutionContext context
160+
) {
161+
Automaton automaton = pattern().createAutomaton(caseInsensitive());
162+
return fieldType.automatonQuery(automaton, constantScoreRewrite, context, getLuceneQueryDescription());
155163
}
156164

157-
@Override
158-
public String getLuceneQueryDescription() {
165+
private String getLuceneQueryDescription() {
159166
// we use the information used to create the automaton to describe the query here
160167
String patternDesc = pattern().patternList().stream().map(WildcardPattern::pattern).collect(Collectors.joining("\", \""));
161168
return "LIKE(\"" + patternDesc + "\"), caseInsensitive=" + caseInsensitive();

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/io/stream/ExpressionQueryBuilder.java

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99

1010
import org.apache.lucene.search.MatchNoDocsQuery;
1111
import org.apache.lucene.search.Query;
12-
import org.apache.lucene.util.automaton.Automaton;
1312
import org.elasticsearch.TransportVersion;
1413
import org.elasticsearch.common.Strings;
1514
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
@@ -99,13 +98,11 @@ protected void doXContent(XContentBuilder builder, Params params) throws IOExcep
9998
@Override
10099
protected Query doToQuery(SearchExecutionContext context) throws IOException {
101100
if (expression instanceof TranslationAware translationAware) {
102-
Automaton automaton = translationAware.asLuceneQuery();
103101
MappedFieldType fieldType = context.getFieldType(fieldName);
104102
if (fieldType == null) {
105103
return new MatchNoDocsQuery("Field [" + fieldName + "] does not exist");
106104
}
107-
String description = translationAware.getLuceneQueryDescription();
108-
return fieldType.automatonQuery(automaton, CONSTANT_SCORE_REWRITE, context, description);
105+
return translationAware.asLuceneQuery(fieldType, CONSTANT_SCORE_REWRITE, context);
109106
} else {
110107
throw new UnsupportedOperationException("ExpressionQueryBuilder does not support non-automaton expressions");
111108
}

0 commit comments

Comments
 (0)