[ML][Transform] Filter null objects from field caps request (elastic#…

…62945) If the transform grouping is a script then exclude the field from the source index mappings fields caps request. A null object caused an NPE in the serialisation of FieldCapabilitiesIndexRequest.
palesz · Sep 28, 2020 · 8ccecc3 · 8ccecc3
1 parent b9cd9ec
commit 8ccecc3
Show file tree

Hide file tree

Showing 2 changed files with 17 additions and 5 deletions.
diff --git a/...ransform/src/main/java/org/elasticsearch/xpack/transform/transforms/pivot/SchemaUtil.java b/...ransform/src/main/java/org/elasticsearch/xpack/transform/transforms/pivot/SchemaUtil.java
@@ -25,6 +25,7 @@
 
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -77,6 +78,11 @@ public static void deduceMappings(
             .getGroups()
             .forEach(
                 (destinationFieldName, group) -> {
+                    // skip any fields that use scripts as there will be no source mapping
+                    if (group.getScriptConfig() != null) {
+                        return;
+                    }
+
                     // We will always need the field name for the grouping to create the mapping
                     fieldNamesForGrouping.put(destinationFieldName, group.getField());
                     // Sometimes the group config will supply a desired mapping as well
@@ -107,7 +113,7 @@ public static void deduceMappings(
         getSourceFieldMappings(
             client,
             source,
-            allFieldNames.values().toArray(new String[0]),
+            allFieldNames.values().stream().filter(Objects::nonNull).toArray(String[]::new),
             ActionListener.wrap(
                 sourceMappings -> listener.onResponse(
                     resolveMappings(
@@ -182,7 +188,8 @@ private static Map<String, String> resolveMappings(
             } else if (destinationMapping != null) {
                 targetMapping.put(targetFieldName, destinationMapping);
             } else {
-                logger.warn("Failed to deduce mapping for [{}], fall back to dynamic mapping.", targetFieldName);
+                logger.warn("Failed to deduce mapping for [{}], fall back to dynamic mapping. " +
+                    "Create the destination index with complete mappings first to avoid deducing the mappings", targetFieldName);
             }
         });
 
@@ -192,7 +199,8 @@ private static Map<String, String> resolveMappings(
             if (destinationMapping != null) {
                 targetMapping.put(targetFieldName, destinationMapping);
             } else {
-                logger.warn("Failed to deduce mapping for [{}], fall back to keyword.", targetFieldName);
+                logger.warn("Failed to deduce mapping for [{}], fall back to keyword. " +
+                    "Create the destination index with complete mappings first to avoid deducing the mappings", targetFieldName);
                 targetMapping.put(targetFieldName, KeywordFieldMapper.CONTENT_TYPE);
             }
         });

diff --git a/...a/org/elasticsearch/xpack/transform/transforms/pivot/AggregationSchemaAndResultTests.java b/...a/org/elasticsearch/xpack/transform/transforms/pivot/AggregationSchemaAndResultTests.java
@@ -127,11 +127,13 @@ public void testBasic() throws InterruptedException {
         AggregationConfig aggregationConfig = new AggregationConfig(Collections.emptyMap(), aggs);
         GroupConfig groupConfig = GroupConfigTests.randomGroupConfig();
         PivotConfig pivotConfig = new PivotConfig(groupConfig, aggregationConfig, null);
+        long numGroupsWithoutScripts = groupConfig.getGroups().values().stream()
+            .filter(singleGroupSource -> singleGroupSource.getScriptConfig() == null).count();
 
         this.<Map<String, String>>assertAsync(
             listener -> SchemaUtil.deduceMappings(client, pivotConfig, new String[] { "source-index" }, listener),
             mappings -> {
-                assertEquals(groupConfig.getGroups().size() + 10, mappings.size());
+                assertEquals(numGroupsWithoutScripts + 10, mappings.size());
                 assertEquals("long", mappings.get("max_rating"));
                 assertEquals("double", mappings.get("avg_rating"));
                 assertEquals("long", mappings.get("count_rating"));
@@ -189,11 +191,13 @@ public void testNested() throws InterruptedException {
         AggregationConfig aggregationConfig = new AggregationConfig(Collections.emptyMap(), aggs);
         GroupConfig groupConfig = GroupConfigTests.randomGroupConfig();
         PivotConfig pivotConfig = new PivotConfig(groupConfig, aggregationConfig, null);
+        long numGroupsWithoutScripts = groupConfig.getGroups().values().stream()
+            .filter(singleGroupSource -> singleGroupSource.getScriptConfig() == null).count();
 
         this.<Map<String, String>>assertAsync(
             listener -> SchemaUtil.deduceMappings(client, pivotConfig, new String[] { "source-index" }, listener),
             mappings -> {
-                assertEquals(groupConfig.getGroups().size() + 12, mappings.size());
+                assertEquals(numGroupsWithoutScripts + 12, mappings.size());
                 assertEquals("long", mappings.get("filter_1"));
                 assertEquals("object", mappings.get("filter_2"));
                 assertEquals("long", mappings.get("filter_2.max_drinks_2"));