[7.9][Transform]disable optimizations when using scripts in group_by (#…

…62524) the outcome and we have no query counterpart. Other optimizations for other group_by's are not affected. relates #57332 backport #60724
elastic · Sep 17, 2020 · bc78b03 · bc78b03
1 parent c056b7e
commit bc78b03
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 1 deletion.
diff --git a/...t/java/org/elasticsearch/xpack/core/transform/transforms/pivot/TermsGroupSourceTests.java b/...t/java/org/elasticsearch/xpack/core/transform/transforms/pivot/TermsGroupSourceTests.java
@@ -21,6 +21,11 @@ public static TermsGroupSource randomTermsGroupSource() {
         return new TermsGroupSource(field, scriptConfig);
     }
 
+    public static TermsGroupSource randomTermsGroupSourceNoScript() {
+        String field = randomAlphaOfLengthBetween(1, 20);
+        return new TermsGroupSource(field, null);
+    }
+
     @Override
     protected TermsGroupSource doParseInstance(XContentParser parser) throws IOException {
         return TermsGroupSource.fromXContent(parser, false);

diff --git a/...a/org/elasticsearch/xpack/transform/transforms/pivot/CompositeBucketsChangeCollector.java b/...a/org/elasticsearch/xpack/transform/transforms/pivot/CompositeBucketsChangeCollector.java
@@ -381,6 +381,11 @@ static Map<String, FieldCollector> createFieldCollectors(Map<String, SingleGroup
         Map<String, FieldCollector> fieldCollectors = new HashMap<>();
 
         for (Entry<String, SingleGroupSource> entry : groups.entrySet()) {
+            // skip any fields that use scripts
+            if (entry.getValue().getScriptConfig() != null) {
+                continue;
+            }
+
             switch (entry.getValue().getType()) {
                 case TERMS:
                     fieldCollectors.put(

diff --git a/.../elasticsearch/xpack/transform/transforms/pivot/CompositeBucketsChangeCollectorTests.java b/.../elasticsearch/xpack/transform/transforms/pivot/CompositeBucketsChangeCollectorTests.java
@@ -25,6 +25,7 @@
 import org.elasticsearch.xpack.core.transform.transforms.pivot.GroupConfig;
 import org.elasticsearch.xpack.core.transform.transforms.pivot.GroupConfigTests;
 import org.elasticsearch.xpack.core.transform.transforms.pivot.HistogramGroupSourceTests;
+import org.elasticsearch.xpack.core.transform.transforms.pivot.ScriptConfigTests;
 import org.elasticsearch.xpack.core.transform.transforms.pivot.SingleGroupSource;
 import org.elasticsearch.xpack.core.transform.transforms.pivot.TermsGroupSource;
 import org.elasticsearch.xpack.core.transform.transforms.pivot.TermsGroupSourceTests;
@@ -76,7 +77,7 @@ public void testPageSize() throws IOException {
         assertEquals(10, getCompositeAggregationBuilder(collector.buildChangesQuery(new SearchSourceBuilder(), null, 10)).size());
 
         // a terms group_by is limited by terms query
-        SingleGroupSource termsGroupBy = TermsGroupSourceTests.randomTermsGroupSource();
+        SingleGroupSource termsGroupBy = TermsGroupSourceTests.randomTermsGroupSourceNoScript();
         groups.put("terms", termsGroupBy);
 
         collector = CompositeBucketsChangeCollector.buildChangeCollector(getCompositeAggregation(groups), groups, null);
@@ -138,6 +139,24 @@ public void testTermsFieldCollector() throws IOException {
         assertThat(((TermsQueryBuilder) queryBuilder).values(), containsInAnyOrder("id1", "id2", "id3"));
     }
 
+    public void testNoTermsFieldCollectorForScripts() throws IOException {
+        Map<String, SingleGroupSource> groups = new LinkedHashMap<>();
+
+        // terms with value script
+        SingleGroupSource termsGroupBy = new TermsGroupSource("id", ScriptConfigTests.randomScriptConfig());
+        groups.put("id", termsGroupBy);
+
+        Map<String, FieldCollector> fieldCollectors = CompositeBucketsChangeCollector.createFieldCollectors(groups, null);
+        assertTrue(fieldCollectors.isEmpty());
+
+        // terms with only a script
+        termsGroupBy = new TermsGroupSource(null, ScriptConfigTests.randomScriptConfig());
+        groups.put("id", termsGroupBy);
+
+        fieldCollectors = CompositeBucketsChangeCollector.createFieldCollectors(groups, null);
+        assertTrue(fieldCollectors.isEmpty());
+    }
+
     private static CompositeAggregationBuilder getCompositeAggregation(Map<String, SingleGroupSource> groups) throws IOException {
         CompositeAggregationBuilder compositeAggregation;
         try (XContentBuilder builder = jsonBuilder()) {