default to match_all if query is not given (#38865)

add a default "match_all" query to make query support more visible

Author: Hendrik Muhs

x-pack/plugin/data-frame/src/main/java/org/elasticsearch/xpack/dataframe/transforms/DataFrameIndexer.java

@@ -12,7 +12,6 @@
 import org.elasticsearch.action.search.SearchRequest;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.common.xcontent.XContentBuilder;
-import org.elasticsearch.index.query.MatchAllQueryBuilder;
 import org.elasticsearch.index.query.QueryBuilder;
 import org.elasticsearch.search.aggregations.bucket.composite.CompositeAggregation;
 import org.elasticsearch.xpack.core.dataframe.transform.DataFrameIndexerTransformStats;
@@ -46,8 +45,7 @@ public DataFrameIndexer(Executor executor, AtomicReference<IndexerState> initial
 
     @Override
     protected void onStartJob(long now) {
-        QueryConfig queryConfig = getConfig().getQueryConfig();
-        QueryBuilder queryBuilder = queryConfig != null ? queryConfig.getQuery() : new MatchAllQueryBuilder();
+        QueryBuilder queryBuilder = getConfig().getQueryConfig().getQuery();
 
         pivot = new Pivot(getConfig().getSource(), queryBuilder, getConfig().getPivotConfig());
     }

x-pack/plugin/data-frame/src/main/java/org/elasticsearch/xpack/dataframe/transforms/DataFrameTransformConfig.java

@@ -17,12 +17,14 @@
 import org.elasticsearch.common.xcontent.ToXContentObject;
 import org.elasticsearch.common.xcontent.XContentBuilder;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.index.query.MatchAllQueryBuilder;
 import org.elasticsearch.xpack.core.dataframe.DataFrameField;
 import org.elasticsearch.xpack.core.dataframe.DataFrameMessages;
 import org.elasticsearch.xpack.core.ml.utils.ExceptionsHelper;
 import org.elasticsearch.xpack.dataframe.transforms.pivot.PivotConfig;
 
 import java.io.IOException;
+import java.util.Collections;
 import java.util.Objects;
 
 import static org.elasticsearch.common.xcontent.ConstructingObjectParser.constructorArg;
@@ -57,7 +59,16 @@ private static ConstructingObjectParser<DataFrameTransformConfig, String> create
                     String id = args[0] != null ? (String) args[0] : optionalId;
                     String source = (String) args[1];
                     String dest = (String) args[2];
-                    QueryConfig queryConfig = (QueryConfig) args[3];
+
+                    // default handling: if the user does not specify a query, we default to match_all
+                    QueryConfig queryConfig = null;
+                    if (args[3] == null) {
+                        queryConfig = new QueryConfig(Collections.singletonMap(MatchAllQueryBuilder.NAME, Collections.emptyMap()),
+                                new MatchAllQueryBuilder());
+                    } else {
+                        queryConfig = (QueryConfig) args[3];
+                    }
+
                     PivotConfig pivotConfig = (PivotConfig) args[4];
                     return new DataFrameTransformConfig(id, source, dest, queryConfig, pivotConfig);
                 });
@@ -83,7 +94,7 @@ public DataFrameTransformConfig(final String id,
         this.id = ExceptionsHelper.requireNonNull(id, DataFrameField.ID.getPreferredName());
         this.source = ExceptionsHelper.requireNonNull(source, SOURCE.getPreferredName());
         this.dest = ExceptionsHelper.requireNonNull(dest, DESTINATION.getPreferredName());
-        this.queryConfig = queryConfig;
+        this.queryConfig = ExceptionsHelper.requireNonNull(queryConfig, QUERY.getPreferredName());
         this.pivotConfig = pivotConfig;
 
         // at least one function must be defined

x-pack/plugin/data-frame/src/test/java/org/elasticsearch/xpack/dataframe/transforms/DataFrameTransformConfigTests.java

@@ -6,13 +6,21 @@
 
 package org.elasticsearch.xpack.dataframe.transforms;
 
+import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.io.stream.Writeable.Reader;
+import org.elasticsearch.common.xcontent.DeprecationHandler;
+import org.elasticsearch.common.xcontent.ToXContent;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.XContentFactory;
 import org.elasticsearch.common.xcontent.XContentParser;
+import org.elasticsearch.common.xcontent.XContentType;
 import org.elasticsearch.xpack.dataframe.transforms.pivot.PivotConfigTests;
 import org.junit.Before;
 
 import java.io.IOException;
 
+import static org.elasticsearch.test.TestMatchers.matchesPattern;
+
 public class DataFrameTransformConfigTests extends AbstractSerializingDataFrameTestCase<DataFrameTransformConfig> {
 
     private String transformId;
@@ -54,4 +62,38 @@ protected DataFrameTransformConfig createTestInstance() {
     protected Reader<DataFrameTransformConfig> instanceReader() {
         return DataFrameTransformConfig::new;
     }
+
+    public void testDefaultMatchAll( ) throws IOException {
+        String pivotTransform = "{"
+                + " \"source\" : \"src\","
+                + " \"dest\" : \"dest\","
+                + " \"pivot\" : {"
+                + " \"group_by\": [ {"
+                + "   \"id\": {"
+                + "     \"terms\": {"
+                + "       \"field\": \"id\""
+                + "} } } ],"
+                + " \"aggs\": {"
+                + "   \"avg\": {"
+                + "     \"avg\": {"
+                + "       \"field\": \"points\""
+                + "} } } } }";
+
+        DataFrameTransformConfig dataFrameTransformConfig = createDataFrameTransformConfigFromString(pivotTransform, "test_match_all");
+        assertNotNull(dataFrameTransformConfig.getQueryConfig());
+        assertTrue(dataFrameTransformConfig.getQueryConfig().isValid());
+
+        try (XContentBuilder xContentBuilder = XContentFactory.jsonBuilder()) {
+            XContentBuilder content = dataFrameTransformConfig.toXContent(xContentBuilder, ToXContent.EMPTY_PARAMS);
+            String pivotTransformWithIdAndDefaults = Strings.toString(content);
+
+            assertThat(pivotTransformWithIdAndDefaults, matchesPattern(".*\"match_all\"\\s*:\\s*\\{\\}.*"));
+        }
+    }
+
+    private DataFrameTransformConfig createDataFrameTransformConfigFromString(String json, String id) throws IOException {
+        final XContentParser parser = XContentType.JSON.xContent().createParser(xContentRegistry(),
+                DeprecationHandler.THROW_UNSUPPORTED_OPERATION, json);
+        return DataFrameTransformConfig.fromXContent(parser, id, false);
+    }
 }

x-pack/plugin/data-frame/src/test/java/org/elasticsearch/xpack/dataframe/transforms/QueryConfigTests.java

@@ -101,6 +101,36 @@ public void testFailOnStrictPassOnLenient() throws IOException {
         }
     }
 
+    public void testFailOnEmptyQuery() throws IOException {
+        String source = "";
+
+        // lenient, passes but reports invalid
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {
+            QueryConfig query = QueryConfig.fromXContent(parser, true);
+            assertFalse(query.isValid());
+        }
+
+        // strict throws
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {
+            expectThrows(IllegalArgumentException.class, () -> QueryConfig.fromXContent(parser, false));
+        }
+    }
+
+    public void testFailOnEmptyQueryClause() throws IOException {
+        String source = "{}";
+
+        // lenient, passes but reports invalid
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {
+            QueryConfig query = QueryConfig.fromXContent(parser, true);
+            assertFalse(query.isValid());
+        }
+
+        // strict throws
+        try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {
+            expectThrows(IllegalArgumentException.class, () -> QueryConfig.fromXContent(parser, false));
+        }
+    }
+
     public void testDeprecation() throws IOException {
         String source = "{\"" + MockDeprecatedQueryBuilder.NAME + "\" : {}}";
         try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) {