From a14ec8e64b35fc54d052344c336da22e56792533 Mon Sep 17 00:00:00 2001 From: David Roberts Date: Fri, 6 Oct 2023 11:13:35 +0100 Subject: [PATCH] [ML] Change aggregation exception types AggregationExecutionException maps to a 500 REST status, and should not be used for situations where end user choices (for example query timerange or parameters) caused the exception. This change converts these exceptions to IllegalArgumentException without the ML plugin. --- .../java/org/elasticsearch/xpack/ml/aggs/MlAggsHelper.java | 3 +-- .../ml/aggs/categorization/CategorizationBytesRefHash.java | 3 +-- .../ml/aggs/correlation/BucketCorrelationAggregator.java | 3 +-- .../xpack/ml/aggs/correlation/CountCorrelationFunction.java | 5 ++--- .../elasticsearch/xpack/ml/aggs/heuristic/PValueScore.java | 5 +---- .../xpack/ml/aggs/inference/InferencePipelineAggregator.java | 5 ++--- .../xpack/ml/aggs/kstest/BucketCountKSTestAggregator.java | 2 +- 7 files changed, 9 insertions(+), 17 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/MlAggsHelper.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/MlAggsHelper.java index 876474e08485a..780841880a6c1 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/MlAggsHelper.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/MlAggsHelper.java @@ -8,7 +8,6 @@ package org.elasticsearch.xpack.ml.aggs; import org.elasticsearch.search.aggregations.Aggregation; -import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.Aggregations; import org.elasticsearch.search.aggregations.InternalMultiBucketAggregation; import org.elasticsearch.search.aggregations.InvalidAggregationPathException; @@ -76,7 +75,7 @@ public static Optional extractDoubleBucketedValues( bucketCount++; continue; } - throw new AggregationExecutionException( + throw new IllegalArgumentException( "missing or invalid bucket value found for path [" + bucketPath + "] in bucket [" diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/categorization/CategorizationBytesRefHash.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/categorization/CategorizationBytesRefHash.java index 23d95b4ba0f7f..58feb24480f87 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/categorization/CategorizationBytesRefHash.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/categorization/CategorizationBytesRefHash.java @@ -11,7 +11,6 @@ import org.elasticsearch.common.logging.LoggerMessageFormat; import org.elasticsearch.common.util.BytesRefHash; import org.elasticsearch.core.Releasable; -import org.elasticsearch.search.aggregations.AggregationExecutionException; class CategorizationBytesRefHash implements Releasable { @@ -49,7 +48,7 @@ int put(BytesRef bytesRef) { return (int) (-1L - hash); } if (hash > Integer.MAX_VALUE) { - throw new AggregationExecutionException( + throw new IllegalArgumentException( LoggerMessageFormat.format( "more than [{}] unique terms encountered. " + "Consider restricting the documents queried or adding [{}] in the {} configuration", diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/correlation/BucketCorrelationAggregator.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/correlation/BucketCorrelationAggregator.java index 1d90f879526e2..02386acbd6134 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/correlation/BucketCorrelationAggregator.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/correlation/BucketCorrelationAggregator.java @@ -8,7 +8,6 @@ package org.elasticsearch.xpack.ml.aggs.correlation; import org.elasticsearch.search.DocValueFormat; -import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.AggregationReduceContext; import org.elasticsearch.search.aggregations.Aggregations; import org.elasticsearch.search.aggregations.InternalAggregation; @@ -45,7 +44,7 @@ public InternalAggregation doReduce(Aggregations aggregations, AggregationReduce ) .orElse(null); if (bucketPathValue == null) { - throw new AggregationExecutionException( + throw new IllegalArgumentException( "unable to find valid bucket values in path [" + bucketsPaths()[0] + "] for agg [" + name() + "]" ); } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/correlation/CountCorrelationFunction.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/correlation/CountCorrelationFunction.java index 8908fe303aa01..87cfd4bae7015 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/correlation/CountCorrelationFunction.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/correlation/CountCorrelationFunction.java @@ -9,7 +9,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.PipelineAggregationBuilder; import org.elasticsearch.search.aggregations.pipeline.MovingFunctions; import org.elasticsearch.xcontent.ConstructingObjectParser; @@ -99,7 +98,7 @@ public boolean equals(Object obj) { @Override public double execute(CountCorrelationIndicator y) { if (indicator.getExpectations().length != y.getExpectations().length) { - throw new AggregationExecutionException( + throw new IllegalArgumentException( "value lengths do not match; indicator.expectations [" + indicator.getExpectations().length + "] and number of buckets [" @@ -136,7 +135,7 @@ public double execute(CountCorrelationIndicator y) { } final double weight = MovingFunctions.sum(y.getExpectations()) / indicator.getDocCount(); if (weight > 1.0) { - throw new AggregationExecutionException( + throw new IllegalArgumentException( "doc_count of indicator must be larger than the total count of the correlating values indicator count [" + indicator.getDocCount() + "] correlating value total count [" diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/heuristic/PValueScore.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/heuristic/PValueScore.java index 402f9d2eb9d22..5cb9bf543fd19 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/heuristic/PValueScore.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/heuristic/PValueScore.java @@ -10,7 +10,6 @@ import org.apache.commons.math3.util.FastMath; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; -import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.bucket.terms.heuristic.NXYSignificanceHeuristic; import org.elasticsearch.search.aggregations.bucket.terms.heuristic.SignificanceHeuristic; import org.elasticsearch.xcontent.ConstructingObjectParser; @@ -175,9 +174,7 @@ public double getScore(long subsetFreq, long subsetSize, long supersetFreq, long || docsContainTermInClass > Long.MAX_VALUE || allDocsNotInClass > Long.MAX_VALUE || docsContainTermNotInClass > Long.MAX_VALUE) { - throw new AggregationExecutionException( - "too many documents in background and foreground sets, further restrict sets for execution" - ); + throw new IllegalArgumentException("too many documents in background and foreground sets, further restrict sets for execution"); } double v1 = new LongBinomialDistribution((long) allDocsInClass, docsContainTermInClass / allDocsInClass).logProbability( diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/inference/InferencePipelineAggregator.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/inference/InferencePipelineAggregator.java index c94eacad6fb86..ea01f07146ea6 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/inference/InferencePipelineAggregator.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/inference/InferencePipelineAggregator.java @@ -8,7 +8,6 @@ package org.elasticsearch.xpack.ml.aggs.inference; import org.elasticsearch.inference.InferenceResults; -import org.elasticsearch.search.aggregations.AggregationExecutionException; import org.elasticsearch.search.aggregations.AggregationReduceContext; import org.elasticsearch.search.aggregations.InternalAggregation; import org.elasticsearch.search.aggregations.InternalAggregations; @@ -132,7 +131,7 @@ public static Object resolveBucketValue( return bucket.getProperty(agg.getName(), aggPathsList); } - private static AggregationExecutionException invalidAggTypeError(String aggPath, Object propertyValue) { + private static IllegalArgumentException invalidAggTypeError(String aggPath, Object propertyValue) { String msg = AbstractPipelineAggregationBuilder.BUCKETS_PATH_FIELD.getPreferredName() + " must reference either a number value, a single value numeric metric aggregation or a string: got [" @@ -143,6 +142,6 @@ private static AggregationExecutionException invalidAggTypeError(String aggPath, + "] at aggregation [" + aggPath + "]"; - return new AggregationExecutionException(msg); + return new IllegalArgumentException(msg); } } diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/kstest/BucketCountKSTestAggregator.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/kstest/BucketCountKSTestAggregator.java index c23335a121e70..518b76aae3732 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/kstest/BucketCountKSTestAggregator.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/aggs/kstest/BucketCountKSTestAggregator.java @@ -237,7 +237,7 @@ public InternalAggregation doReduce(Aggregations aggregations, AggregationReduce } ); if (maybeBucketsValue.isPresent() == false) { - throw new AggregationExecutionException( + throw new IllegalArgumentException( "unable to find valid bucket values in bucket path [" + bucketsPaths()[0] + "] for agg [" + name() + "]" ); }