From f3ea5b888f33219a5b50124b6131374b575ca719 Mon Sep 17 00:00:00 2001 From: Dennis Oelkers Date: Fri, 8 Nov 2024 09:15:12 +0100 Subject: [PATCH] Sorting pivot values numerically when field is numeric. --- .../pivot/buckets/ESValuesHandler.java | 39 +++++++++++++--- .../pivot/buckets/OSValuesHandler.java | 45 +++++++++++++++---- 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/graylog-storage-elasticsearch7/src/main/java/org/graylog/storage/elasticsearch7/views/searchtypes/pivot/buckets/ESValuesHandler.java b/graylog-storage-elasticsearch7/src/main/java/org/graylog/storage/elasticsearch7/views/searchtypes/pivot/buckets/ESValuesHandler.java index 686358e80817..eab79466abdf 100644 --- a/graylog-storage-elasticsearch7/src/main/java/org/graylog/storage/elasticsearch7/views/searchtypes/pivot/buckets/ESValuesHandler.java +++ b/graylog-storage-elasticsearch7/src/main/java/org/graylog/storage/elasticsearch7/views/searchtypes/pivot/buckets/ESValuesHandler.java @@ -23,6 +23,8 @@ import org.graylog.plugins.views.search.aggregations.MissingBucketConstants; import org.graylog.plugins.views.search.searchtypes.pivot.BucketSpec; import org.graylog.plugins.views.search.searchtypes.pivot.Pivot; +import org.graylog.plugins.views.search.searchtypes.pivot.PivotSort; +import org.graylog.plugins.views.search.searchtypes.pivot.SortSpec; import org.graylog.plugins.views.search.searchtypes.pivot.buckets.Values; import org.graylog.plugins.views.search.searchtypes.pivot.buckets.ValuesBucketOrdering; import org.graylog.shaded.elasticsearch7.org.elasticsearch.index.query.BoolQueryBuilder; @@ -44,6 +46,7 @@ import javax.annotation.Nonnull; import java.util.Collection; import java.util.List; +import java.util.Optional; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -53,19 +56,21 @@ public class ESValuesHandler extends ESPivotBucketSpecHandler { private static final String KEY_SEPARATOR_PHRASE = " + \"" + KEY_SEPARATOR_CHARACTER + "\" + "; private static final String AGG_NAME = "agg"; private static final ImmutableList MISSING_BUCKET_KEYS = ImmutableList.of(MissingBucketConstants.MISSING_BUCKET_NAME); - private static final BucketOrder defaultOrder = BucketOrder.count(false); + public static final BucketOrder DEFAULT_ORDER = BucketOrder.count(false); + public static final String SORT_HELPER = "sort_helper"; @Nonnull @Override public CreatedAggregations doCreateAggregation(Direction direction, String name, Pivot pivot, Values bucketSpec, ESGeneratedQueryContext queryContext, Query query) { - final List ordering = orderListForPivot(pivot, queryContext, defaultOrder); + final List ordering = orderListForPivot(pivot, queryContext, DEFAULT_ORDER); final int limit = bucketSpec.limit(); final List orderedBuckets = ValuesBucketOrdering.orderFields(bucketSpec.fields(), pivot.sort()); - final AggregationBuilder termsAggregation = createTerms(orderedBuckets, ordering, limit); + final var termsAggregation = createTerms(orderedBuckets, ordering, limit); + + applyOrdering(pivot, termsAggregation, orderedBuckets, ordering, queryContext); final FiltersAggregationBuilder filterAggregation = createFilter(name, orderedBuckets, bucketSpec.skipEmptyValues()) .subAggregation(termsAggregation); - return CreatedAggregations.create(filterAggregation, termsAggregation, List.of(termsAggregation, filterAggregation)); } @@ -79,7 +84,7 @@ private FiltersAggregationBuilder createFilter(String name, List fields, } - private AggregationBuilder createTerms(List valueBuckets, List ordering, int limit) { + private TermsAggregationBuilder createTerms(List valueBuckets, List ordering, int limit) { return createScriptedTerms(valueBuckets, ordering, limit); } @@ -103,6 +108,30 @@ private Script scriptForPivots(Collection pivots) { return new Script(scriptSource); } + private TermsAggregationBuilder applyOrdering(Pivot pivot, TermsAggregationBuilder terms, List buckets, List ordering, ESGeneratedQueryContext queryContext) { + return sortsOnNumericPivotField(pivot, queryContext) + .map(pivotSort -> terms + .subAggregation(AggregationBuilders.max(SORT_HELPER).field(pivotSort.field())) + .order(BucketOrder.aggregation(SORT_HELPER, SortSpec.Direction.Ascending.equals(pivotSort.direction())))) + .orElseGet(() -> terms + .order(ordering.isEmpty() ? List.of(DEFAULT_ORDER) : ordering)); + } + + private Optional sortsOnNumericPivotField(Pivot pivot, ESGeneratedQueryContext queryContext) { + return Optional.ofNullable(pivot.sort()) + .filter(sorts -> sorts.size() == 1) + .map(sorts -> sorts.get(0)) + .filter(sort -> sort instanceof PivotSort) + .map(sort -> (PivotSort) sort) + .filter(pivotSort -> queryContext.fieldType(pivot.effectiveStreams(), pivotSort.field()) + .filter(this::isNumericFieldType) + .isPresent()); + } + + private boolean isNumericFieldType(String fieldType) { + return fieldType.equals("long") || fieldType.equals("double") || fieldType.equals("float"); + } + @Override public Stream extractBuckets(Pivot pivot, BucketSpec bucketSpec, PivotBucket initialBucket) { var values = (Values) bucketSpec; diff --git a/graylog-storage-opensearch2/src/main/java/org/graylog/storage/opensearch2/views/searchtypes/pivot/buckets/OSValuesHandler.java b/graylog-storage-opensearch2/src/main/java/org/graylog/storage/opensearch2/views/searchtypes/pivot/buckets/OSValuesHandler.java index 678504584e32..c192ddf7bb8b 100644 --- a/graylog-storage-opensearch2/src/main/java/org/graylog/storage/opensearch2/views/searchtypes/pivot/buckets/OSValuesHandler.java +++ b/graylog-storage-opensearch2/src/main/java/org/graylog/storage/opensearch2/views/searchtypes/pivot/buckets/OSValuesHandler.java @@ -23,6 +23,8 @@ import org.graylog.plugins.views.search.aggregations.MissingBucketConstants; import org.graylog.plugins.views.search.searchtypes.pivot.BucketSpec; import org.graylog.plugins.views.search.searchtypes.pivot.Pivot; +import org.graylog.plugins.views.search.searchtypes.pivot.PivotSort; +import org.graylog.plugins.views.search.searchtypes.pivot.SortSpec; import org.graylog.plugins.views.search.searchtypes.pivot.buckets.Values; import org.graylog.plugins.views.search.searchtypes.pivot.buckets.ValuesBucketOrdering; import org.graylog.shaded.opensearch2.org.opensearch.index.query.BoolQueryBuilder; @@ -44,6 +46,7 @@ import javax.annotation.Nonnull; import java.util.Collection; import java.util.List; +import java.util.Optional; import java.util.function.Function; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -53,15 +56,18 @@ public class OSValuesHandler extends OSPivotBucketSpecHandler { private static final String KEY_SEPARATOR_PHRASE = " + \"" + KEY_SEPARATOR_CHARACTER + "\" + "; private static final String AGG_NAME = "agg"; private static final ImmutableList MISSING_BUCKET_KEYS = ImmutableList.of(MissingBucketConstants.MISSING_BUCKET_NAME); - private static final BucketOrder defaultOrder = BucketOrder.count(false); + public static final BucketOrder DEFAULT_ORDER = BucketOrder.count(false); + public static final String SORT_HELPER = "sort_helper"; @Nonnull @Override public CreatedAggregations doCreateAggregation(Direction direction, String name, Pivot pivot, Values bucketSpec, OSGeneratedQueryContext queryContext, Query query) { - final List ordering = orderListForPivot(pivot, queryContext, defaultOrder); + final List ordering = orderListForPivot(pivot, queryContext, DEFAULT_ORDER); final int limit = bucketSpec.limit(); final List orderedBuckets = ValuesBucketOrdering.orderFields(bucketSpec.fields(), pivot.sort()); - final AggregationBuilder termsAggregation = createTerms(orderedBuckets, ordering, limit, bucketSpec.skipEmptyValues()); + final var termsAggregation = createTerms(orderedBuckets, ordering, limit, bucketSpec.skipEmptyValues()); + + applyOrdering(pivot, termsAggregation, orderedBuckets, ordering, queryContext); final FiltersAggregationBuilder filterAggregation = createFilter(name, orderedBuckets, bucketSpec.skipEmptyValues()) .subAggregation(termsAggregation); @@ -77,15 +83,14 @@ private FiltersAggregationBuilder createFilter(String name, List bucketS .otherBucket(true); } - private AggregationBuilder createTerms(List valueBuckets, List ordering, int limit, boolean skipEmptyValues) { - return createScriptedTerms(valueBuckets, ordering, limit); + private TermsAggregationBuilder createTerms(List valueBuckets, List ordering, int limit, boolean skipEmptyValues) { + return createScriptedTerms(valueBuckets, limit); } - private TermsAggregationBuilder createScriptedTerms(List buckets, List ordering, int limit) { + private TermsAggregationBuilder createScriptedTerms(List buckets, int limit) { return AggregationBuilders.terms(AGG_NAME) .script(scriptForPivots(buckets)) - .size(limit) - .order(ordering.isEmpty() ? List.of(BucketOrder.count(false)) : ordering); + .size(limit); } private Script scriptForPivots(Collection pivots) { @@ -101,6 +106,30 @@ private Script scriptForPivots(Collection pivots) { return new Script(scriptSource); } + private TermsAggregationBuilder applyOrdering(Pivot pivot, TermsAggregationBuilder terms, List buckets, List ordering, OSGeneratedQueryContext queryContext) { + return sortsOnNumericPivotField(pivot, queryContext) + .map(pivotSort -> terms + .subAggregation(AggregationBuilders.max(SORT_HELPER).field(pivotSort.field())) + .order(BucketOrder.aggregation(SORT_HELPER, SortSpec.Direction.Ascending.equals(pivotSort.direction())))) + .orElseGet(() -> terms + .order(ordering.isEmpty() ? List.of(DEFAULT_ORDER) : ordering)); + } + + private Optional sortsOnNumericPivotField(Pivot pivot, OSGeneratedQueryContext queryContext) { + return Optional.ofNullable(pivot.sort()) + .filter(sorts -> sorts.size() == 1) + .map(sorts -> sorts.get(0)) + .filter(sort -> sort instanceof PivotSort) + .map(sort -> (PivotSort) sort) + .filter(pivotSort -> queryContext.fieldType(pivot.effectiveStreams(), pivotSort.field()) + .filter(this::isNumericFieldType) + .isPresent()); + } + + private boolean isNumericFieldType(String fieldType) { + return fieldType.equals("long") || fieldType.equals("double") || fieldType.equals("float"); + } + @Override public Stream extractBuckets(Pivot pivot, BucketSpec bucketSpecs, PivotBucket initialBucket) { var values = (Values) bucketSpecs;