Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ML-DataFrame] default to match_all if query is not given #38865

Merged
merged 1 commit into from
Feb 14, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
default to match_all if query is not given
Hendrik Muhs committed Feb 13, 2019
commit 6b310d610f0f2b3dbcddbe0c7b0a25b7f2d53fcc
Original file line number Diff line number Diff line change
@@ -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());
}
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
@@ -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)) {