Merge remote-tracking branch 'upstream/main' into exclude_vector_source

Author: jimczi

docs/changelog/132167.yaml

@@ -0,0 +1,6 @@
+pr: 132167
+summary: Deal with internally created IN in a different way for EQL
+area: EQL
+type: bug
+issues:
+ - 118621

docs/changelog/132320.yaml

@@ -0,0 +1,6 @@
+pr: 132320
+summary: "Aggs: Add validation to Bucket script pipeline agg"
+area: Aggregations
+type: bug
+issues:
+ - 132272

modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/bucket_script.yml

@@ -340,3 +340,31 @@ top level fails:
                 buckets_path:
                   b: b
                 script: params.b + 12
+
+---
+invalid parent aggregation:
+  - requires:
+      capabilities:
+        - method: POST
+          path: /_search
+          capabilities: [ bucket_script_parent_multi_bucket_error ]
+      test_runner_features: [capabilities]
+      reason: "changed error 500 to 400"
+  - do:
+      catch: /Expected a multi bucket aggregation but got \[InternalFilter\] for aggregation \[d\]/
+      search:
+        body:
+          aggs:
+            a:
+              filter:
+                term:
+                  a: 1
+              aggs:
+                b:
+                  sum:
+                    field: b
+                d:
+                  bucket_script:
+                    buckets_path:
+                      b: b
+                    script: params.b + 12

server/src/main/java/org/elasticsearch/rest/action/search/SearchCapabilities.java

@@ -55,6 +55,7 @@ private SearchCapabilities() {}
     private static final String FIELD_EXISTS_QUERY_FOR_TEXT_FIELDS_NO_INDEX_OR_DV = "field_exists_query_for_text_fields_no_index_or_dv";
     private static final String UPDATE_FIELD_TO_BBQ_DISK = "update_field_to_bbq_disk";
     private static final String KNN_FILTER_ON_NESTED_FIELDS_CAPABILITY = "knn_filter_on_nested_fields";
+    private static final String BUCKET_SCRIPT_PARENT_MULTI_BUCKET_ERROR = "bucket_script_parent_multi_bucket_error";
     private static final String EXCLUDE_SOURCE_VECTORS_SETTING = "exclude_source_vectors_setting";
 
     public static final Set<String> CAPABILITIES;
@@ -81,8 +82,9 @@ private SearchCapabilities() {}
         capabilities.add(DENSE_VECTOR_UPDATABLE_BBQ);
         capabilities.add(FIELD_EXISTS_QUERY_FOR_TEXT_FIELDS_NO_INDEX_OR_DV);
         capabilities.add(UPDATE_FIELD_TO_BBQ_DISK);
-        capabilities.add(EXCLUDE_SOURCE_VECTORS_SETTING);
         capabilities.add(KNN_FILTER_ON_NESTED_FIELDS_CAPABILITY);
+        capabilities.add(BUCKET_SCRIPT_PARENT_MULTI_BUCKET_ERROR);
+        capabilities.add(EXCLUDE_SOURCE_VECTORS_SETTING);
         CAPABILITIES = Set.copyOf(capabilities);
     }
 }

server/src/main/java/org/elasticsearch/search/aggregations/pipeline/BucketScriptPipelineAggregator.java

@@ -21,6 +21,7 @@
 import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
+import java.util.Locale;
 import java.util.Map;
 
 import static org.elasticsearch.search.aggregations.pipeline.BucketHelpers.resolveBucketValue;
@@ -47,10 +48,24 @@ public class BucketScriptPipelineAggregator extends PipelineAggregator {
     }
 
     @Override
+    @SuppressWarnings({ "rawtypes", "unchecked" })
     public InternalAggregation reduce(InternalAggregation aggregation, AggregationReduceContext reduceContext) {
-        @SuppressWarnings({ "rawtypes", "unchecked" })
-        InternalMultiBucketAggregation<InternalMultiBucketAggregation, InternalMultiBucketAggregation.InternalBucket> originalAgg =
-            (InternalMultiBucketAggregation<InternalMultiBucketAggregation, InternalMultiBucketAggregation.InternalBucket>) aggregation;
+
+        InternalMultiBucketAggregation<InternalMultiBucketAggregation, InternalMultiBucketAggregation.InternalBucket> originalAgg;
+
+        if (aggregation instanceof InternalMultiBucketAggregation multiBucketAggregation) {
+            originalAgg = multiBucketAggregation;
+        } else {
+            throw new IllegalArgumentException(
+                String.format(
+                    Locale.ROOT,
+                    "Expected a multi bucket aggregation but got [%s] for aggregation [%s]",
+                    aggregation.getClass().getSimpleName(),
+                    name()
+                )
+            );
+        }
+
         List<? extends InternalMultiBucketAggregation.InternalBucket> buckets = originalAgg.getBuckets();
 
         BucketAggregationScript.Factory factory = reduceContext.scriptService().compile(script, BucketAggregationScript.CONTEXT);

server/src/test/java/org/elasticsearch/search/aggregations/pipeline/BucketScriptAggregatorTests.java

@@ -31,7 +31,9 @@
 import org.elasticsearch.script.ScriptModule;
 import org.elasticsearch.script.ScriptService;
 import org.elasticsearch.script.ScriptType;
+import org.elasticsearch.search.aggregations.AggregationBuilder;
 import org.elasticsearch.search.aggregations.AggregatorTestCase;
+import org.elasticsearch.search.aggregations.bucket.filter.FilterAggregationBuilder;
 import org.elasticsearch.search.aggregations.bucket.filter.FiltersAggregationBuilder;
 import org.elasticsearch.search.aggregations.bucket.filter.InternalFilters;
 import org.elasticsearch.search.aggregations.bucket.terms.TermsAggregationBuilder;
@@ -102,8 +104,42 @@ public void testScript() throws IOException {
         );
     }
 
+    public void testNonMultiBucketParent() {
+        MappedFieldType fieldType = new NumberFieldMapper.NumberFieldType("number_field", NumberFieldMapper.NumberType.INTEGER);
+        MappedFieldType fieldType1 = new KeywordFieldMapper.KeywordFieldType("the_field");
+
+        FilterAggregationBuilder filter = new FilterAggregationBuilder("placeholder", new MatchAllQueryBuilder()).subAggregation(
+            new TermsAggregationBuilder("the_terms").userValueTypeHint(ValueType.STRING)
+                .field("the_field")
+                .subAggregation(new AvgAggregationBuilder("the_avg").field("number_field"))
+        )
+            .subAggregation(
+                new BucketScriptPipelineAggregationBuilder(
+                    "bucket_script",
+                    Collections.singletonMap("the_avg", "the_terms['test1']>the_avg.value"),
+                    new Script(ScriptType.INLINE, MockScriptEngine.NAME, SCRIPT_NAME, Collections.emptyMap())
+                )
+            );
+
+        assertThrows(
+            "Expected a multi bucket aggregation but got [InternalFilter] for aggregation [bucket_script]",
+            IllegalArgumentException.class,
+            () -> testCase(filter, new MatchAllDocsQuery(), iw -> {
+                Document doc = new Document();
+                doc.add(new SortedSetDocValuesField("the_field", new BytesRef("test1")));
+                doc.add(new SortedNumericDocValuesField("number_field", 19));
+                iw.addDocument(doc);
+
+                doc = new Document();
+                doc.add(new SortedSetDocValuesField("the_field", new BytesRef("test2")));
+                doc.add(new SortedNumericDocValuesField("number_field", 55));
+                iw.addDocument(doc);
+            }, f -> fail("This shouldn't be called"), fieldType, fieldType1)
+        );
+    }
+
     private void testCase(
-        FiltersAggregationBuilder aggregationBuilder,
+        AggregationBuilder aggregationBuilder,
         Query query,
         CheckedConsumer<RandomIndexWriter, IOException> buildIndex,
         Consumer<InternalFilters> verify,

x-pack/plugin/eql/qa/common/build.gradle

@@ -8,3 +8,10 @@ dependencies {
   // TOML parser for EqlActionIT tests
   api 'io.ous:jtoml:2.0.0'
 }
+
+tasks.register("loadTestData", JavaExec) {
+  group = "Execution"
+  description = "Loads EQL Spec Tests data on a running stand-alone instance"
+  classpath = sourceSets.main.runtimeClasspath
+  mainClass = "org.elasticsearch.test.eql.DataLoader"
+}

x-pack/plugin/eql/qa/common/src/main/java/org/elasticsearch/test/eql/DataLoader.java

@@ -76,47 +76,69 @@ private static Map<String, String[]> getReplacementPatterns() {
     public static void main(String[] args) throws IOException {
         main = true;
         try (RestClient client = RestClient.builder(new HttpHost("localhost", 9200)).build()) {
-            loadDatasetIntoEs(client, DataLoader::createParser);
+            loadDatasetIntoEsWithIndexCreator(client, DataLoader::createParser, (restClient, indexName, indexMapping) -> {
+                // don't use ESRestTestCase methods here or, if you do, test running the main method before making the change
+                StringBuilder jsonBody = new StringBuilder("{");
+                jsonBody.append("\"settings\":{\"number_of_shards\":1},");
+                jsonBody.append("\"mappings\":");
+                jsonBody.append(indexMapping);
+                jsonBody.append("}");
+
+                Request request = new Request("PUT", "/" + indexName);
+                request.setJsonEntity(jsonBody.toString());
+                restClient.performRequest(request);
+            });
         }
     }
 
     public static void loadDatasetIntoEs(RestClient client, CheckedBiFunction<XContent, InputStream, XContentParser, IOException> p)
         throws IOException {
+        loadDatasetIntoEsWithIndexCreator(client, p, (restClient, indexName, indexMapping) -> {
+            ESRestTestCase.createIndex(restClient, indexName, Settings.builder().put("number_of_shards", 1).build(), indexMapping, null);
+        });
+    }
+
+    private static void loadDatasetIntoEsWithIndexCreator(
+        RestClient client,
+        CheckedBiFunction<XContent, InputStream, XContentParser, IOException> p,
+        IndexCreator indexCreator
+    ) throws IOException {
 
         //
         // Main Index
         //
-        load(client, TEST_INDEX, null, DataLoader::timestampToUnixMillis, p);
+        load(client, TEST_INDEX, null, DataLoader::timestampToUnixMillis, p, indexCreator);
         //
         // Aux Index
         //
-        load(client, TEST_EXTRA_INDEX, null, null, p);
+        load(client, TEST_EXTRA_INDEX, null, null, p, indexCreator);
         //
         // Date_Nanos index
         //
         // The data for this index is loaded from the same endgame-140.data sample, only having the mapping for @timestamp changed: the
         // chosen Windows filetime timestamps (2017+) can coincidentally also be readily used as nano-resolution unix timestamps (1973+).
         // There are mixed values with and without nanos precision so that the filtering is properly tested for both cases.
-        load(client, TEST_NANOS_INDEX, TEST_INDEX, DataLoader::timestampToUnixNanos, p);
-        load(client, TEST_SAMPLE, null, null, p);
+        load(client, TEST_NANOS_INDEX, TEST_INDEX, DataLoader::timestampToUnixNanos, p, indexCreator);
+        load(client, TEST_SAMPLE, null, null, p, indexCreator);
         //
         // missing_events index
         //
-        load(client, TEST_MISSING_EVENTS_INDEX, null, null, p);
-        load(client, TEST_SAMPLE_MULTI, null, null, p);
+        load(client, TEST_MISSING_EVENTS_INDEX, null, null, p, indexCreator);
+        load(client, TEST_SAMPLE_MULTI, null, null, p, indexCreator);
         //
         // index with a runtime field ("broken", type long) that causes shard failures.
         // the rest of the mapping is the same as TEST_INDEX
         //
-        load(client, TEST_SHARD_FAILURES_INDEX, null, DataLoader::timestampToUnixMillis, p);
+        load(client, TEST_SHARD_FAILURES_INDEX, null, DataLoader::timestampToUnixMillis, p, indexCreator);
     }
 
     private static void load(
         RestClient client,
         String indexNames,
         String dataName,
         Consumer<Map<String, Object>> datasetTransform,
-        CheckedBiFunction<XContent, InputStream, XContentParser, IOException> p
+        CheckedBiFunction<XContent, InputStream, XContentParser, IOException> p,
+        IndexCreator indexCreator
     ) throws IOException {
         String[] splitNames = indexNames.split(",");
         for (String indexName : splitNames) {
@@ -130,15 +152,11 @@ private static void load(
             if (data == null) {
                 throw new IllegalArgumentException("Cannot find resource " + name);
             }
-            createTestIndex(client, indexName, readMapping(mapping));
+            indexCreator.createIndex(client, indexName, readMapping(mapping));
             loadData(client, indexName, datasetTransform, data, p);
         }
     }
 
-    private static void createTestIndex(RestClient client, String indexName, String mapping) throws IOException {
-        ESRestTestCase.createIndex(client, indexName, Settings.builder().put("number_of_shards", 1).build(), mapping, null);
-    }
-
     /**
      * Reads the mapping file, ignoring comments and replacing placeholders for random types.
      */
@@ -236,4 +254,8 @@ private static XContentParser createParser(XContent xContent, InputStream data)
         NamedXContentRegistry contentRegistry = new NamedXContentRegistry(ClusterModule.getNamedXWriteables());
         return xContent.createParser(contentRegistry, LoggingDeprecationHandler.INSTANCE, data);
     }
+
+    private interface IndexCreator {
+        void createIndex(RestClient client, String indexName, String mapping) throws IOException;
+    }
 }

x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/optimizer/Optimizer.java

@@ -44,7 +44,6 @@
 import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BinaryComparisonSimplification;
 import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanFunctionEqualsElimination;
 import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.BooleanSimplification;
-import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.CombineDisjunctionsToIn;
 import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.ConstantFolding;
 import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.LiteralsOnTheRight;
 import org.elasticsearch.xpack.ql.optimizer.OptimizerRules.OptimizerRule;
@@ -252,6 +251,14 @@ protected Expression maybeSimplifyNegatable(Expression e) {
 
     }
 
+    static class CombineDisjunctionsToIn extends org.elasticsearch.xpack.ql.optimizer.OptimizerRules.CombineDisjunctionsToIn {
+
+        @Override
+        protected boolean shouldValidateIn() {
+            return true;
+        }
+    }
+
     static class PruneFilters extends org.elasticsearch.xpack.ql.optimizer.OptimizerRules.PruneFilters {
 
         @Override

x-pack/plugin/eql/src/test/resources/querytranslator_tests.txt

@@ -123,6 +123,90 @@ process where process_name in ("python.exe", "SMSS.exe", "explorer.exe")
 "terms":{"process_name":["python.exe","SMSS.exe","explorer.exe"],
 ;
 
+mutipleOrEquals_As_InTranslation1
+process where process_name == "python.exe" or process_name == "SMSS.exe" or process_name == "explorer.exe"
+;
+"terms":{"process_name":["python.exe","SMSS.exe","explorer.exe"],
+;
+
+multipleOrAndEquals_As_InTranslation
+process where process_name == "python.exe" and process_name == "SMSS.exe" or process_name == "explorer.exe" or process_name == "test.exe"
+;
+{"bool":{"should":[{"bool":{"must":[{"term":{"process_name":{"value":"python.exe"}}},{"term":{"process_name":{"value":"SMSS.exe"}}}],"boost":1.0}},{"terms":{"process_name":["explorer.exe","test.exe"],"boost":1.0}}],"boost":1.0}}
+;
+
+mutipleOrEquals_As_InTranslation2
+process where source_address == "123.12.1.1" or (opcode == 123 or opcode == 127)
+;
+{"bool":{"should":[{"term":{"source_address":{"value":"123.12.1.1"}}},{"terms":{"opcode":[123,127],"boost":1.0}}],"boost":1.0}}
+;
+
+mutipleOrEquals_As_InTranslation3
+process where (source_address == "123.12.1.1" or source_address == "127.0.0.1") and (opcode == 123 or opcode == 127)
+;
+{"bool":{"should":[{"term":{"source_address":{"value":"123.12.1.1"}}},{"term":{"source_address":{"value":"127.0.0.1"}}}],"boost":1.0}},{"terms":{"opcode":[123,127],"boost":1.0}}
+;
+
+mutipleOrEquals_As_InTranslation4
+process where (source_address == "123.12.1.1" or source_address == "127.0.0.1") and (opcode == 123 or opcode == 127)
+;
+"must":[{"bool":{"should":[{"term":{"source_address":{"value":"123.12.1.1"}}},{"term":{"source_address":{"value":"127.0.0.1"}}}],"boost":1.0}},{"terms":{"opcode":[123,127],"boost":1.0}},{"term":{"event.category":{"value":"process"}}}]
+;
+
+multipleOrIncompatibleTypes1
+process where process_name == "python.exe" or process_name == 2 or process_name == "3"
+;
+{"bool":{"should":[{"term":{"process_name":{"value":"python.exe"}}},{"term":{"process_name":{"value":2}}},{"term":{"process_name":{"value":"3"}}}],"boost":1.0}}
+;
+
+multipleOrIncompatibleTypes2
+process where process_name == "1" or process_name == 2 or process_name == "3"
+;
+{"bool":{"should":[{"term":{"process_name":{"value":"1"}}},{"term":{"process_name":{"value":2}}},{"term":{"process_name":{"value":"3"}}}],"boost":1.0}}
+;
+
+multipleOrIncompatibleTypes3
+process where process_name == 1.2 or process_name == 2 or process_name == "3"
+;
+{"bool":{"should":[{"term":{"process_name":{"value":1.2}}},{"term":{"process_name":{"value":2}}},{"term":{"process_name":{"value":"3"}}}],"boost":1.0}}
+;
+
+// this query as an equivalent with
+// process where process_name in (1.2, 2, 3)
+// will result in a user error: 1st argument of [process_name in (1.2, 2, 3)] must be [keyword], found value [1.2] type [double]
+multipleOrIncompatibleTypes4
+process where process_name == 1.2 or process_name == 2 or process_name == 3
+;
+{"bool":{"should":[{"term":{"process_name":{"value":1.2}}},{"term":{"process_name":{"value":2}}},{"term":{"process_name":{"value":3}}}],"boost":1.0}}
+;
+
+// this query as an equivalent with
+// process where source_address in ("123.12.1.1", "123.12.1.2")
+// will result in a user error: 1st argument of [source_address in ("123.12.1.1", "123.12.1.2")] must be [ip], found value ["123.12.1.1"] type [keyword]
+multipleOrIncompatibleTypes5
+process where source_address == "123.12.1.1" or source_address == "123.12.1.2"
+;
+{"bool":{"should":[{"term":{"source_address":{"value":"123.12.1.1"}}},{"term":{"source_address":{"value":"123.12.1.2"}}}],"boost":1.0}}
+;
+
+multipleOrIncompatibleTypes6
+process where source_address == "123.12.1.1" or source_address == concat("123.12.","1.2")
+;
+{"bool":{"should":[{"term":{"source_address":{"value":"123.12.1.1"}}},{"term":{"source_address":{"value":"123.12.1.2"}}}],"boost":1.0}}
+;
+
+multipleOrIncompatibleTypes7
+process where source_address == "123.12.1.1" and (source_address == "123.12.1.2" or source_address >= "127.0.0.1")
+;
+"must":[{"term":{"source_address":{"value":"123.12.1.1"}}},{"bool":{"should":[{"term":{"source_address":{"value":"123.12.1.2"}}},{"range":{"source_address":{"gte":"127.0.0.1","boost":1.0}}}],"boost":1.0}},{"term":{"event.category":{"value":"process"}}}]
+;
+
+multipleOrIncompatibleTypes8
+process where source_address == "123.12.1.1" and (source_address == "123.12.1.2" or source_address == "127.0.0.1")
+;
+"must":[{"term":{"source_address":{"value":"123.12.1.1"}}},{"bool":{"should":[{"term":{"source_address":{"value":"123.12.1.2"}}},{"term":{"source_address":{"value":"127.0.0.1"}}}],"boost":1.0}},{"term":{"event.category":{"value":"process"}}}]
+;
+
 inFilterWithScripting
 process where substring(command_line, 5) in ("test*","best")
 ;