[Transform] maintain a list of unsupported aggregations in transforms (#52190) (#52222)

add a list of unsupported aggs in transforms and create a test that fails if a new aggregation is
added. Limitation: works only if a new agg is added to either the core or a known plugin
(Analytics, MatrixAggregation).
This commit is contained in:
Hendrik Muhs 2020-02-12 07:48:04 +01:00 committed by GitHub
parent dd14210689
commit edaf6d1f79
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 93 additions and 0 deletions

View File

@ -13,6 +13,8 @@ dependencies {
compileOnly project(path: xpackModule('core'), configuration: 'default')
testCompile project(path: xpackModule('core'), configuration: 'testArtifacts')
testCompile project(path: xpackModule('analytics'), configuration: 'runtime')
testCompile project(path: ':modules:aggs-matrix-stats', configuration: 'runtime')
}
// xpack modules are installed in real clusters as the meta plugin, so

View File

@ -12,6 +12,7 @@ import org.elasticsearch.xpack.transform.utils.OutputFieldNameConverter;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Set;
@ -32,6 +33,50 @@ public final class Aggregations {
public static final String GEO_SHAPE = "geo_shape";
public static final String GEO_POINT = "geo_point";
/*
* List of currently unsupported aggregations (not group_by) in transform.
*
* The only purpose of this list is to track which aggregations should be added to transform and assert if new
* aggregations are added.
*
* Created a new aggs?
*
* Please add it to the list (sorted) together with a comment containing a link to the created github issue.
*/
private static final List<String> UNSUPPORTED_AGGS = Arrays.asList(
"adjacency_matrix",
"auto_date_histogram",
"boxplot", // https://github.com/elastic/elasticsearch/issues/52189
"composite", // DONT because it makes no sense
"date_histogram",
"date_range",
"diversified_sampler",
"extended_stats", // https://github.com/elastic/elasticsearch/issues/51925
"filter", // https://github.com/elastic/elasticsearch/issues/52151
"filters",
"geo_distance",
"geohash_grid",
"geotile_grid",
"global",
"histogram",
"ip_range",
"matrix_stats",
"median_absolute_deviation",
"missing",
"nested",
"percentile_ranks",
"range",
"rare_terms",
"reverse_nested",
"sampler",
"significant_terms", // https://github.com/elastic/elasticsearch/issues/51073
"significant_text",
"stats", // https://github.com/elastic/elasticsearch/issues/51925
"string_stats", // https://github.com/elastic/elasticsearch/issues/51925
"terms", // https://github.com/elastic/elasticsearch/issues/51073
"top_hits"
);
private Aggregations() {}
/**
@ -79,10 +124,19 @@ public final class Aggregations {
.map(AggregationType::name)
.collect(Collectors.toSet());
private static Set<String> aggregationsNotSupported = UNSUPPORTED_AGGS.stream()
.map(agg -> agg.toUpperCase(Locale.ROOT))
.collect(Collectors.toSet());
public static boolean isSupportedByTransform(String aggregationType) {
return aggregationSupported.contains(aggregationType.toUpperCase(Locale.ROOT));
}
// only for testing
static boolean isUnSupportedByTransform(String aggregationType) {
return aggregationsNotSupported.contains(aggregationType.toUpperCase(Locale.ROOT));
}
public static boolean isDynamicMapping(String targetMapping) {
return DYNAMIC.equals(targetMapping);
}

View File

@ -6,7 +6,17 @@
package org.elasticsearch.xpack.transform.transforms.pivot;
import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
import org.elasticsearch.common.settings.Settings;
import org.elasticsearch.search.SearchModule;
import org.elasticsearch.search.aggregations.AggregationBuilder;
import org.elasticsearch.search.aggregations.matrix.MatrixAggregationPlugin;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.analytics.AnalyticsPlugin;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;
public class AggregationsTests extends ESTestCase {
public void testResolveTargetMapping() {
@ -68,4 +78,31 @@ public class AggregationsTests extends ESTestCase {
assertEquals("double", Aggregations.resolveTargetMapping("percentiles", null));
assertEquals("double", Aggregations.resolveTargetMapping("percentiles", "int"));
}
public void testAggregationsVsTransforms() {
// Note: if a new plugin is added, it must be added here
SearchModule searchModule = new SearchModule(Settings.EMPTY, false, Arrays.asList(new AnalyticsPlugin(Settings.EMPTY),
new MatrixAggregationPlugin()));
List<NamedWriteableRegistry.Entry> namedWriteables = searchModule.getNamedWriteables();
List<String> aggregationNames = namedWriteables.stream()
.filter(namedWritable -> namedWritable.categoryClass.equals(AggregationBuilder.class))
.map(namedWritable -> namedWritable.name)
.collect(Collectors.toList());
for (String aggregationName : aggregationNames) {
assertTrue(
"The following aggregation is unknown to transform: ["
+ aggregationName
+ "]. If this is a newly added aggregation, "
+ "please open an issue to add transform support for it. Afterwards add \""
+ aggregationName
+ "\" to the list in "
+ Aggregations.class.getName()
+ ". Thanks!",
Aggregations.isSupportedByTransform(aggregationName) || Aggregations.isUnSupportedByTransform(aggregationName)
);
}
}
}