Skip to content

Commit

Permalink
[ML] Change aggregation exception types
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
droberts195 committed Oct 6, 2023
1 parent e1fa3ef commit a14ec8e
Show file tree
Hide file tree
Showing 7 changed files with 9 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -76,7 +75,7 @@ public static Optional<DoubleBucketValues> extractDoubleBucketedValues(
bucketCount++;
continue;
}
throw new AggregationExecutionException(
throw new IllegalArgumentException(
"missing or invalid bucket value found for path ["
+ bucketPath
+ "] in bucket ["
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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() + "]"
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ["
Expand Down Expand Up @@ -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 ["
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 ["
Expand All @@ -143,6 +142,6 @@ private static AggregationExecutionException invalidAggTypeError(String aggPath,
+ "] at aggregation ["
+ aggPath
+ "]";
return new AggregationExecutionException(msg);
return new IllegalArgumentException(msg);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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() + "]"
);
}
Expand Down

0 comments on commit a14ec8e

Please sign in to comment.