Address code review feedback part 2 (still needs UT with the flag set to false)

julian-elastic · julian-elastic · commit f0b6e5a862dd · 2025-06-30T23:30:34.000-04:00
diff --git a/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/WildcardQuery.java b/x-pack/plugin/esql-core/src/main/java/org/elasticsearch/xpack/esql/core/querydsl/query/WildcardQuery.java
@@ -16,16 +16,14 @@ public class WildcardQuery extends Query {
 
     private final String field, query;
     private final boolean caseInsensitive;
+    private final boolean forceStringMatch;
 
-    public WildcardQuery(Source source, String field, String query) {
-        this(source, field, query, false);
-    }
-
-    public WildcardQuery(Source source, String field, String query, boolean caseInsensitive) {
+    public WildcardQuery(Source source, String field, String query, boolean caseInsensitive, boolean forceStringMatch) {
         super(source);
         this.field = field;
         this.query = query;
         this.caseInsensitive = caseInsensitive;
+        this.forceStringMatch = forceStringMatch;
     }
 
     public String field() {
@@ -42,7 +40,7 @@ public Boolean caseInsensitive() {
 
     @Override
     protected QueryBuilder asBuilder() {
-        WildcardQueryBuilder wb = new WildcardQueryBuilder(field, query, true);
+        WildcardQueryBuilder wb = new WildcardQueryBuilder(field, query, forceStringMatch);
         // ES does not allow case_insensitive to be set to "false", it should be either "true" or not specified
         return caseInsensitive == false ? wb : wb.caseInsensitive(caseInsensitive);
     }
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/ConfigurationTestUtils.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/ConfigurationTestUtils.java
@@ -72,7 +72,8 @@ public static Configuration randomConfiguration(String query, Map<String, Map<St
             profile,
             tables,
             System.nanoTime(),
-            false
+            false,
+            true // stringLikeOnIndex, default to true for tests
         );
     }
 
diff --git a/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java b/x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/EsqlTestUtils.java
@@ -429,7 +429,8 @@ public static Configuration configuration(QueryPragmas pragmas, String query) {
             false,
             TABLES,
             System.nanoTime(),
-            false
+            false,
+            true // stringLikeOnIndex, default to true for tests
         );
     }
 
@@ -863,7 +864,7 @@ static Version randomVersion() {
     }
 
     public static WildcardLike wildcardLike(Expression left, String exp) {
-        return new WildcardLike(EMPTY, left, new WildcardPattern(exp));
+        return new WildcardLike(EMPTY, left, new WildcardPattern(exp), false, TEST_CFG);
     }
 
     public static RLike rlike(Expression left, String exp) {
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWith.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/EndsWith.java
@@ -151,7 +151,7 @@ public Query asQuery(LucenePushdownPredicates pushdownPredicates, TranslatorHand
         // TODO: Get the real FoldContext here
         var wildcardQuery = "*" + QueryParser.escape(BytesRefs.toString(suffix.fold(FoldContext.small())));
 
-        return new WildcardQuery(source(), fieldName, wildcardQuery);
+        return new WildcardQuery(source(), fieldName, wildcardQuery, false, false);
     }
 
     @Override
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWith.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/StartsWith.java
@@ -148,7 +148,7 @@ public Query asQuery(LucenePushdownPredicates pushdownPredicates, TranslatorHand
         // TODO: Get the real FoldContext here
         var wildcardQuery = QueryParser.escape(BytesRefs.toString(prefix.fold(FoldContext.small()))) + "*";
 
-        return new WildcardQuery(source(), fieldName, wildcardQuery);
+        return new WildcardQuery(source(), fieldName, wildcardQuery, false, false);
     }
 
     @Override
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/regex/WildcardLike.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/regex/WildcardLike.java
@@ -23,6 +23,7 @@
 import org.elasticsearch.xpack.esql.io.stream.PlanStreamInput;
 import org.elasticsearch.xpack.esql.optimizer.rules.physical.local.LucenePushdownPredicates;
 import org.elasticsearch.xpack.esql.planner.TranslatorHandler;
+import org.elasticsearch.xpack.esql.session.Configuration;
 
 import java.io.IOException;
 
@@ -33,6 +34,7 @@ public class WildcardLike extends RegexMatch<WildcardPattern> {
         WildcardLike::new
     );
     public static final String NAME = "LIKE";
+    public final Configuration configuration;
 
     @FunctionInfo(returnType = "boolean", description = """
         Use `LIKE` to filter data based on string patterns using wildcards. `LIKE`
@@ -60,21 +62,24 @@ also act on a constant (literal) expression. The right-hand side of the operator
     public WildcardLike(
         Source source,
         @Param(name = "str", type = { "keyword", "text" }, description = "A literal expression.") Expression left,
-        @Param(name = "pattern", type = { "keyword", "text" }, description = "Pattern.") WildcardPattern pattern
+        @Param(name = "pattern", type = { "keyword", "text" }, description = "Pattern.") WildcardPattern pattern,
+        @Param(name = "configuration", type = "configuration", description = "configuration") Configuration configuration
     ) {
-        this(source, left, pattern, false);
+        this(source, left, pattern, false, configuration);
     }
 
-    public WildcardLike(Source source, Expression left, WildcardPattern pattern, boolean caseInsensitive) {
+    public WildcardLike(Source source, Expression left, WildcardPattern pattern, boolean caseInsensitive, Configuration configuration) {
         super(source, left, pattern, caseInsensitive);
+        this.configuration = configuration;
     }
 
     private WildcardLike(StreamInput in) throws IOException {
         this(
             Source.readFrom((PlanStreamInput) in),
             in.readNamedWriteable(Expression.class),
             new WildcardPattern(in.readString()),
-            deserializeCaseInsensitivity(in)
+            deserializeCaseInsensitivity(in),
+            Configuration.readConfigurationHelper(in)
         );
     }
 
@@ -84,6 +89,7 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeNamedWriteable(field());
         out.writeString(pattern().pattern());
         serializeCaseInsensitivity(out);
+        Configuration.writeConfigurationHelper(out, configuration);
     }
 
     @Override
@@ -98,12 +104,12 @@ public String getWriteableName() {
 
     @Override
     protected NodeInfo<WildcardLike> info() {
-        return NodeInfo.create(this, WildcardLike::new, field(), pattern(), caseInsensitive());
+        return NodeInfo.create(this, WildcardLike::new, field(), pattern(), caseInsensitive(), configuration);
     }
 
     @Override
     protected WildcardLike replaceChild(Expression newLeft) {
-        return new WildcardLike(source(), newLeft, pattern(), caseInsensitive());
+        return new WildcardLike(source(), newLeft, pattern(), caseInsensitive(), configuration);
     }
 
     @Override
@@ -120,6 +126,12 @@ public Query asQuery(LucenePushdownPredicates pushdownPredicates, TranslatorHand
 
     // TODO: see whether escaping is needed
     private Query translateField(String targetFieldName) {
-        return new WildcardQuery(source(), targetFieldName, pattern().asLuceneWildcard(), caseInsensitive());
+        return new WildcardQuery(
+            source(),
+            targetFieldName,
+            pattern().asLuceneWildcard(),
+            caseInsensitive(),
+            configuration.stringLikeOnIndex()
+        );
     }
 }
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/regex/WildcardLikeList.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/string/regex/WildcardLikeList.java
@@ -72,7 +72,7 @@ public WildcardLikeList(StreamInput in) throws IOException {
             in.readNamedWriteable(Expression.class),
             new WildcardPatternList(in),
             deserializeCaseInsensitivity(in),
-            readConfiguration(in)
+            Configuration.readConfigurationHelper(in)
         );
     }
 
@@ -82,32 +82,7 @@ public void writeTo(StreamOutput out) throws IOException {
         out.writeNamedWriteable(field());
         pattern().writeTo(out);
         serializeCaseInsensitivity(out);
-        writeConfiguration(out);
-
-    }
-
-    static Configuration readConfiguration(StreamInput in) throws IOException {
-        if (in.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIXED_INDEX_LIKE)) {
-            boolean hasConfiguration = in.readBoolean();
-            if (hasConfiguration) {
-                return Configuration.readFrom(in);
-            } else {
-                return null; // For backward compatibility, configuration is not serialized before this version
-            }
-        } else {
-            return null; // For backward compatibility, configuration is not serialized before this version
-        }
-    }
-
-    void writeConfiguration(StreamOutput out) throws IOException {
-        if (out.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIXED_INDEX_LIKE)) {
-            boolean hasConfiguration = configuration != null;
-            out.writeBoolean(hasConfiguration);
-            if (hasConfiguration) {
-                configuration.writeTo(out);
-            }
-        }
-        // else don't write configuration for backward compatibility
+        Configuration.writeConfigurationHelper(out, configuration);
     }
 
     @Override
@@ -146,6 +121,11 @@ public Translatable translatable(LucenePushdownPredicates pushdownPredicates) {
      */
     @Override
     public Query asQuery(LucenePushdownPredicates pushdownPredicates, TranslatorHandler handler) {
+        if(configuration != null && configuration.stringLikeOnIndex() == false) {
+            throw new IllegalArgumentException(
+                "LIKE with LIST cannot be used with string_like_on_index enabled. Use LIKE instead."
+            );
+        }
         var field = field();
         LucenePushdownPredicates.checkIsPushableAttribute(field);
         String targetFieldName = handler.nameOf(field instanceof FieldAttribute fa ? fa.exactAttribute() : field);
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceStringCasingWithInsensitiveRegexMatch.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/ReplaceStringCasingWithInsensitiveRegexMatch.java
@@ -47,7 +47,7 @@ private static Expression insensitiveRegexMatch(RegexMatch<? extends StringPatte
                 wildcardLike.source(),
                 unwrapCase(wildcardLike.field()),
                 wildcardLike.pattern(),
-                true
+                wildcardLike.configuration
             );
             default -> regexMatch;
         };
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/ExpressionBuilder.java
@@ -762,7 +762,7 @@ public Expression visitLikeExpression(EsqlBaseParser.LikeExpressionContext ctx)
         Literal patternLiteral = visitString(ctx.string());
         try {
             WildcardPattern pattern = new WildcardPattern(BytesRefs.toString(patternLiteral.fold(FoldContext.small())));
-            WildcardLike result = new WildcardLike(source, left, pattern);
+            WildcardLike result = new WildcardLike(source, left, pattern, configuration);
             return ctx.NOT() == null ? result : new Not(source, result);
         } catch (InvalidArgumentException e) {
             throw new ParsingException(source, "Invalid pattern for LIKE [{}]: [{}]", patternLiteral, e.getMessage());
@@ -779,7 +779,7 @@ public Expression visitLikeListExpression(EsqlBaseParser.LikeListExpressionConte
             .toList();
         // for now we will use the old WildcardLike function for one argument case to allow compatibility in mixed version deployments
         Expression e = wildcardPatterns.size() == 1
-            ? new WildcardLike(source, left, wildcardPatterns.getFirst())
+            ? new WildcardLike(source, left, wildcardPatterns.getFirst(), configuration)
             : new WildcardLikeList(source, left, new WildcardPatternList(wildcardPatterns), configuration);
         return ctx.NOT() == null ? e : new Not(source, e);
     }
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/EsqlPlugin.java
@@ -116,6 +116,13 @@ public class EsqlPlugin extends Plugin implements ActionPlugin, ExtensiblePlugin
         Setting.Property.Dynamic
     );
 
+    public static final Setting<Boolean> ESQL_STRING_LIKE_ON_INDEX = Setting.boolSetting(
+        "esql.query.string_like_on_index",
+        true,
+        Setting.Property.NodeScope,
+        Setting.Property.Dynamic
+    );
+
     public static final Setting<TimeValue> ESQL_QUERYLOG_THRESHOLD_WARN_SETTING = Setting.timeSetting(
         "esql.querylog.threshold.warn",
         TimeValue.timeValueMillis(-1),
@@ -263,7 +270,8 @@ public List<Setting<?>> getSettings() {
             ESQL_QUERYLOG_THRESHOLD_WARN_SETTING,
             ESQL_QUERYLOG_INCLUDE_USER_SETTING,
             DEFAULT_DATA_PARTITIONING,
-            STORED_FIELDS_SEQUENTIAL_PROPORTION
+            STORED_FIELDS_SEQUENTIAL_PROPORTION,
+            ESQL_STRING_LIKE_ON_INDEX
         );
     }
 
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/TransportEsqlQueryAction.java
@@ -86,6 +86,7 @@ public class TransportEsqlQueryAction extends HandledTransportAction<EsqlQueryRe
     private final UsageService usageService;
     private final TransportActionServices services;
     private volatile boolean defaultAllowPartialResults;
+    private volatile boolean defaultESQLStringMatchOnIndex;
 
     @Inject
     @SuppressWarnings("this-escape")
@@ -181,6 +182,10 @@ public TransportEsqlQueryAction(
         defaultAllowPartialResults = EsqlPlugin.QUERY_ALLOW_PARTIAL_RESULTS.get(clusterService.getSettings());
         clusterService.getClusterSettings()
             .addSettingsUpdateConsumer(EsqlPlugin.QUERY_ALLOW_PARTIAL_RESULTS, v -> defaultAllowPartialResults = v);
+
+        defaultESQLStringMatchOnIndex = EsqlPlugin.ESQL_STRING_LIKE_ON_INDEX.get(clusterService.getSettings());
+        clusterService.getClusterSettings()
+            .addSettingsUpdateConsumer(EsqlPlugin.ESQL_STRING_LIKE_ON_INDEX, v -> defaultESQLStringMatchOnIndex = v);
     }
 
     @Override
@@ -233,7 +238,8 @@ private void innerExecute(Task task, EsqlQueryRequest request, ActionListener<Es
             request.profile(),
             request.tables(),
             System.nanoTime(),
-            request.allowPartialResults()
+            request.allowPartialResults(),
+            defaultESQLStringMatchOnIndex
         );
         String sessionId = sessionID(task);
         // async-query uses EsqlQueryTask, so pull the EsqlExecutionInfo out of the task
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/Configuration.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/Configuration.java
@@ -55,6 +55,7 @@ public class Configuration implements Writeable {
 
     private final boolean profile;
     private final boolean allowPartialResults;
+    private final boolean stringLikeOnIndex;
 
     private final Map<String, Map<String, Column>> tables;
     private final long queryStartTimeNanos;
@@ -71,7 +72,8 @@ public Configuration(
         boolean profile,
         Map<String, Map<String, Column>> tables,
         long queryStartTimeNanos,
-        boolean allowPartialResults
+        boolean allowPartialResults,
+        boolean stringLikeOnIndex
     ) {
         this.zoneId = zi.normalized();
         this.now = ZonedDateTime.now(Clock.tick(Clock.system(zoneId), Duration.ofNanos(1)));
@@ -87,6 +89,7 @@ public Configuration(
         assert tables != null;
         this.queryStartTimeNanos = queryStartTimeNanos;
         this.allowPartialResults = allowPartialResults;
+        this.stringLikeOnIndex = stringLikeOnIndex;
     }
 
     public Configuration(BlockStreamInput in) throws IOException {
@@ -120,6 +123,12 @@ public Configuration(BlockStreamInput in) throws IOException {
         } else {
             this.allowPartialResults = false;
         }
+        if (in.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIXED_INDEX_LIKE)) {
+            this.stringLikeOnIndex = in.readBoolean();
+        } else {
+            this.stringLikeOnIndex = false;
+        }
+
     }
 
     @Override
@@ -148,6 +157,9 @@ public void writeTo(StreamOutput out) throws IOException {
             || out.getTransportVersion().isPatchFrom(TransportVersions.ESQL_SUPPORT_PARTIAL_RESULTS_BACKPORT_8_19)) {
             out.writeBoolean(allowPartialResults);
         }
+        if (out.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIXED_INDEX_LIKE)) {
+            out.writeBoolean(stringLikeOnIndex);
+        }
     }
 
     public ZoneId zoneId() {
@@ -230,6 +242,10 @@ public boolean allowPartialResults() {
         return allowPartialResults;
     }
 
+    public boolean stringLikeOnIndex() {
+        return stringLikeOnIndex;
+    }
+
     private static void writeQuery(StreamOutput out, String query) throws IOException {
         if (query.length() > QUERY_COMPRESS_THRESHOLD_CHARS) { // compare on chars to avoid UTF-8 encoding unless actually required
             out.writeBoolean(true);
@@ -323,4 +339,28 @@ public static Configuration readFrom(StreamInput in) throws IOException {
         return new Configuration(blockStreamInput);
     }
 
+    public static Configuration readConfigurationHelper(StreamInput in) throws IOException {
+        if (in.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIXED_INDEX_LIKE)) {
+            boolean hasConfiguration = in.readBoolean();
+            if (hasConfiguration) {
+                return Configuration.readFrom(in);
+            } else {
+                return null; // For backward compatibility, configuration is not serialized before this version
+            }
+        } else {
+            return null; // For backward compatibility, configuration is not serialized before this version
+        }
+    }
+
+    public static void writeConfigurationHelper(StreamOutput out, Configuration configuration) throws IOException {
+        if (out.getTransportVersion().onOrAfter(TransportVersions.ESQL_FIXED_INDEX_LIKE)) {
+            boolean hasConfiguration = configuration != null;
+            out.writeBoolean(hasConfiguration);
+            if (hasConfiguration) {
+                configuration.writeTo(out);
+            }
+        }
+        // else don't write configuration for backward compatibility
+    }
+
 }

Original file line number	Diff line number	Diff line change
`@@ -72,7 +72,8 @@ public static Configuration randomConfiguration(String query, Map<String, Map<St`
`72`	`72`	`profile,`
`73`	`73`	`tables,`
`74`	`74`	`System.nanoTime(),`
`75`		`- false`
	`75`	`+ false,`
	`76`	`+ true // stringLikeOnIndex, default to true for tests`
`76`	`77`	`);`
`77`	`78`	`}`
`78`	`79`
Original file line number	Diff line number	Diff line change
`@@ -429,7 +429,8 @@ public static Configuration configuration(QueryPragmas pragmas, String query) {`
`429`	`429`	`false,`
`430`	`430`	`TABLES,`
`431`	`431`	`System.nanoTime(),`
`432`		`- false`
	`432`	`+ false,`
	`433`	`+ true // stringLikeOnIndex, default to true for tests`
`433`	`434`	`);`
`434`	`435`	`}`
`435`	`436`
`@@ -863,7 +864,7 @@ static Version randomVersion() {`
`863`	`864`	`}`
`864`	`865`
`865`	`866`	`public static WildcardLike wildcardLike(Expression left, String exp) {`
`866`		`- return new WildcardLike(EMPTY, left, new WildcardPattern(exp));`
	`867`	`+ return new WildcardLike(EMPTY, left, new WildcardPattern(exp), false, TEST_CFG);`
`867`	`868`	`}`
`868`	`869`
`869`	`870`	`public static RLike rlike(Expression left, String exp) {`
Original file line number	Diff line number	Diff line change
`@@ -151,7 +151,7 @@ public Query asQuery(LucenePushdownPredicates pushdownPredicates, TranslatorHand`
`151`	`151`	`// TODO: Get the real FoldContext here`
`152`	`152`	`var wildcardQuery = "*" + QueryParser.escape(BytesRefs.toString(suffix.fold(FoldContext.small())));`
`153`	`153`
`154`		`- return new WildcardQuery(source(), fieldName, wildcardQuery);`
	`154`	`+ return new WildcardQuery(source(), fieldName, wildcardQuery, false, false);`
`155`	`155`	`}`
`156`	`156`
`157`	`157`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -148,7 +148,7 @@ public Query asQuery(LucenePushdownPredicates pushdownPredicates, TranslatorHand`
`148`	`148`	`// TODO: Get the real FoldContext here`
`149`	`149`	`var wildcardQuery = QueryParser.escape(BytesRefs.toString(prefix.fold(FoldContext.small()))) + "*";`
`150`	`150`
`151`		`- return new WildcardQuery(source(), fieldName, wildcardQuery);`
	`151`	`+ return new WildcardQuery(source(), fieldName, wildcardQuery, false, false);`
`152`	`152`	`}`
`153`	`153`
`154`	`154`	`@Override`