Skip to content

Commit

Permalink
remap a few exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
not-napoleon committed Oct 10, 2023
1 parent eb70dee commit 3e52b82
Show file tree
Hide file tree
Showing 6 changed files with 56 additions and 30 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

package org.elasticsearch.search.aggregations;

import org.elasticsearch.search.aggregations.pipeline.AbstractPipelineAggregationBuilder;
import org.elasticsearch.search.aggregations.support.ValuesSource;

import java.util.Optional;
Expand All @@ -16,7 +17,6 @@
* Collection of helper methods for what to throw in common aggregation error scenarios.
*/
public class AggregationErrors {
private static String parentType;

private AggregationErrors() {}

Expand Down Expand Up @@ -68,7 +68,7 @@ public static RuntimeException rateWithoutDateHistogram(String name) {
* @param position - optional, for multisource aggregations. Indicates the position of the field causing the problem.
* @return - an appropriate exception
*/
public static RuntimeException reduceTypeMissmatch(String aggregationName, Optional<Integer> position) {
public static RuntimeException reduceTypeMismatch(String aggregationName, Optional<Integer> position) {
String fieldString;
if (position.isPresent()) {
fieldString = "the field in position" + position.get().toString();
Expand Down Expand Up @@ -104,6 +104,15 @@ public static RuntimeException unsupportedMultivalue() {
);
}

/**
* Indicates the given values source is not suitable for use in a multivalued aggregation. This is not retryable.
* @param source a string describing the Values Source
* @return an appropriate exception
*/
public static RuntimeException unsupportedMultivalueValuesSource(String source) {
throw new IllegalArgumentException("ValuesSource type " + source + "is not supported for multi-valued aggregation");
}

/**
* Indicates an attempt to use date rounding on a non-date values source
* @param typeName - name of the type we're attempting to round
Expand All @@ -112,4 +121,25 @@ public static RuntimeException unsupportedMultivalue() {
public static RuntimeException unsupportedRounding(String typeName) {
return new IllegalArgumentException("can't round a [" + typeName + "]");
}

/**
* Indicates that an aggregation path (e.g. from a pipeline agg) references an aggregation of the wrong type, for example
* attempting to take a cumulative cardinality of something other than a cardinality aggregation.
*
* @param aggPath the path element found to be invalid
* @param got What we actually got; this may be null.
* @param currentAgg The name of the aggregation in question
* @return an appropriate exception
*/
public static RuntimeException incompatibleAggregationType(String aggPath, String got, String currentAgg) {

return new IllegalArgumentException(
aggPath
+ " must reference a cardinality aggregation, got: ["
+ (got == null ? "null" : got)
+ "] at aggregation ["
+ currentAgg
+ "]"
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ public InternalAggregation reduce(List<InternalAggregation> aggregations, Aggreg
if (referenceTerms != null && referenceTerms.getClass().equals(terms.getClass()) == false && terms.canLeadReduction()) {
// control gets into this loop when the same field name against which the query is executed
// is of different types in different indices.
throw AggregationErrors.reduceTypeMissmatch(referenceTerms.getName(), Optional.empty());
throw AggregationErrors.reduceTypeMismatch(referenceTerms.getName(), Optional.empty());
}
otherDocCount[0] += terms.getSumOfOtherDocCounts();
final long thisAggDocCountError = getDocCountError(terms);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ public InternalAggregation reduce(List<InternalAggregation> aggregations, Aggreg
&& terms.getClass().equals(UnmappedRareTerms.class) == false) {
// control gets into this loop when the same field name against which the query is executed
// is of different types in different indices.
throw AggregationErrors.reduceTypeMissmatch(referenceTerms.getName(), Optional.empty());
throw AggregationErrors.reduceTypeMismatch(referenceTerms.getName(), Optional.empty());
}
for (B bucket : terms.getBuckets()) {
List<B> bucketList = buckets.computeIfAbsent(bucket.getKey(), k -> new ArrayList<>());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
package org.elasticsearch.xpack.analytics.cumulativecardinality;

import org.elasticsearch.search.DocValueFormat;
import org.elasticsearch.search.aggregations.AggregationErrors;
import org.elasticsearch.search.aggregations.AggregationExecutionException;
import org.elasticsearch.search.aggregations.AggregationReduceContext;
import org.elasticsearch.search.aggregations.InternalAggregation;
Expand Down Expand Up @@ -80,16 +81,6 @@ private AbstractHyperLogLogPlusPlus resolveBucketValue(
) {
List<String> aggPathsList = AggregationPath.parse(aggPath).getPathElementsAsStringList();
Object propertyValue = bucket.getProperty(agg.getName(), aggPathsList);
if (propertyValue == null) {
// Seems like a user error, change to 400
throw new AggregationExecutionException(
AbstractPipelineAggregationBuilder.BUCKETS_PATH_FIELD.getPreferredName() + " must reference a cardinality aggregation"
);
}

if (propertyValue instanceof InternalCardinality) {
return ((InternalCardinality) propertyValue).getCounts();
}

String currentAggName;
if (aggPathsList.isEmpty()) {
Expand All @@ -98,14 +89,22 @@ private AbstractHyperLogLogPlusPlus resolveBucketValue(
currentAggName = aggPathsList.get(0);
}

throw new AggregationExecutionException(
// seems like a user error, change to 400
AbstractPipelineAggregationBuilder.BUCKETS_PATH_FIELD.getPreferredName()
+ " must reference a cardinality aggregation, got: ["
+ propertyValue.getClass().getSimpleName()
+ "] at aggregation ["
+ currentAggName
+ "]"
if (propertyValue == null) {
throw AggregationErrors.incompatibleAggregationType(
AbstractPipelineAggregationBuilder.BUCKETS_PATH_FIELD.getPreferredName(),
"null",
currentAggName
);
}

if (propertyValue instanceof InternalCardinality) {
return ((InternalCardinality) propertyValue).getCounts();
}

throw AggregationErrors.incompatibleAggregationType(
AbstractPipelineAggregationBuilder.BUCKETS_PATH_FIELD.getPreferredName(),
propertyValue.getClass().getSimpleName(),
currentAggName
);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -196,15 +196,15 @@ public int compare(List<Object> thisTerms, List<Object> otherTerms) {
if (thisTerms.size() != otherTerms.size()) {
// Not clear on how this can happen.
throw new AggregationExecutionException(
"Merging/Reducing the multi_term aggregations failed due to different term list" + " sizes"
"Merging/Reducing the multi_term aggregations failed due to different term list sizes"
);
}
for (int i = 0; i < thisTerms.size(); i++) {
final int res;
try {
res = ((Comparable) thisTerms.get(i)).compareTo(otherTerms.get(i));
} catch (ClassCastException ex) {
throw AggregationErrors.reduceTypeMissmatch("MultiTerms", Optional.empty());
throw AggregationErrors.reduceTypeMismatch("MultiTerms", Optional.empty());
}
if (res != 0) {
return res;
Expand Down Expand Up @@ -465,7 +465,7 @@ private boolean[] needsPromotionToDouble(List<InternalAggregation> aggregations)
}
}
if (hasNonNumber && (hasDouble || hasUnsignedLong || hasLong)) {
throw AggregationErrors.reduceTypeMissmatch(name, Optional.of(i + 1));
throw AggregationErrors.reduceTypeMismatch(name, Optional.of(i + 1));
}
// Promotion to double is required if at least 2 of these 3 conditions are true.
if ((hasDouble ? 1 : 0) + (hasUnsignedLong ? 1 : 0) + (hasLong ? 1 : 0) > 1) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
import org.elasticsearch.common.util.Maps;
import org.elasticsearch.index.fielddata.MultiGeoPointValues;
import org.elasticsearch.index.fielddata.SortedNumericDoubleValues;
import org.elasticsearch.search.aggregations.AggregationExecutionException;
import org.elasticsearch.search.aggregations.AggregationErrors;
import org.elasticsearch.search.aggregations.support.MultiValuesSource;
import org.elasticsearch.search.aggregations.support.ValuesSource;
import org.elasticsearch.search.aggregations.support.ValuesSourceConfig;
Expand All @@ -25,10 +25,7 @@ public GeoLineMultiValuesSource(Map<String, ValuesSourceConfig> valuesSourceConf
for (Map.Entry<String, ValuesSourceConfig> entry : valuesSourceConfigs.entrySet()) {
final ValuesSource valuesSource = entry.getValue().getValuesSource();
if (valuesSource instanceof ValuesSource.Numeric == false && valuesSource instanceof ValuesSource.GeoPoint == false) {
// This will not resolve on retry, should be 400
throw new AggregationExecutionException(
"ValuesSource type " + valuesSource.toString() + "is not supported for multi-valued aggregation"
);
throw AggregationErrors.unsupportedMultivalueValuesSource(valuesSource.toString());
}
values.put(entry.getKey(), valuesSource);
}
Expand Down

0 comments on commit 3e52b82

Please sign in to comment.