#19855 Throw exception when maxBounds greater than minBounds

Throw exception when maxBounds greater than minBounds
This commit is contained in:
Colin Goodheart-Smithe 2016-08-08 13:17:25 +01:00 committed by GitHub
commit bf0e42aaeb
3 changed files with 49 additions and 15 deletions

View File

@ -47,8 +47,8 @@ public class HistogramAggregationBuilder
private double interval;
private double offset = 0;
private double minBound = Double.MAX_VALUE;
private double maxBound = Double.MIN_VALUE;
private double minBound = Double.POSITIVE_INFINITY;
private double maxBound = Double.NEGATIVE_INFINITY;
private InternalOrder order = (InternalOrder) Histogram.Order.KEY_ASC;
private boolean keyed = false;
private long minDocCount = 0;
@ -122,17 +122,24 @@ public class HistogramAggregationBuilder
return maxBound;
}
/** Set extended bounds on this builder: buckets between {@code minBound}
* and {@code maxBound} will be created even if no documents fell into
* these buckets. It is possible to create half-open bounds by providing
* {@link Double#POSITIVE_INFINITY} as a {@code minBound} or
* {@link Double#NEGATIVE_INFINITY} as a {@code maxBound}. */
/**
* Set extended bounds on this builder: buckets between {@code minBound} and
* {@code maxBound} will be created even if no documents fell into these
* buckets.
*
* @throws IllegalArgumentException
* if maxBound is less that minBound, or if either of the bounds
* are not finite.
*/
public HistogramAggregationBuilder extendedBounds(double minBound, double maxBound) {
if (minBound == Double.NEGATIVE_INFINITY) {
throw new IllegalArgumentException("minBound must not be -Infinity, got: " + minBound);
if (Double.isFinite(minBound) == false) {
throw new IllegalArgumentException("minBound must be finite, got: " + minBound);
}
if (maxBound == Double.POSITIVE_INFINITY) {
throw new IllegalArgumentException("maxBound must not be +Infinity, got: " + maxBound);
if (Double.isFinite(maxBound) == false) {
throw new IllegalArgumentException("maxBound must be finite, got: " + maxBound);
}
if (maxBound < minBound) {
throw new IllegalArgumentException("maxBound [" + maxBound + "] must be greater than minBound [" + minBound + "]");
}
this.minBound = minBound;
this.maxBound = maxBound;

View File

@ -860,7 +860,7 @@ public class HistogramIT extends ESIntegTestCase {
return;
}
} catch (Exception e) {
} catch (IllegalArgumentException e) {
if (invalidBoundsError) {
// expected
return;
@ -886,7 +886,6 @@ public class HistogramIT extends ESIntegTestCase {
}
}
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/19833")
public void testEmptyWithExtendedBounds() throws Exception {
int lastDataBucketKey = (numValueBuckets - 1) * interval;
@ -938,7 +937,7 @@ public class HistogramIT extends ESIntegTestCase {
return;
}
} catch (Exception e) {
} catch (IllegalArgumentException e) {
if (invalidBoundsError) {
// expected
return;

View File

@ -23,6 +23,9 @@ import org.elasticsearch.search.aggregations.BaseAggregationTestCase;
import org.elasticsearch.search.aggregations.bucket.histogram.HistogramAggregationBuilder;
import org.elasticsearch.search.aggregations.bucket.histogram.Histogram.Order;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.startsWith;
public class HistogramTests extends BaseAggregationTestCase<HistogramAggregationBuilder> {
@Override
@ -31,7 +34,9 @@ public class HistogramTests extends BaseAggregationTestCase<HistogramAggregation
factory.field(INT_FIELD_NAME);
factory.interval(randomDouble() * 1000);
if (randomBoolean()) {
factory.extendedBounds(randomDouble(), randomDouble());
double minBound = randomDouble();
double maxBound = randomDoubleBetween(minBound, 1, true);
factory.extendedBounds(minBound, maxBound);
}
if (randomBoolean()) {
factory.format("###.##");
@ -74,4 +79,27 @@ public class HistogramTests extends BaseAggregationTestCase<HistogramAggregation
return factory;
}
public void testInvalidBounds() {
HistogramAggregationBuilder factory = new HistogramAggregationBuilder("foo");
factory.field(INT_FIELD_NAME);
factory.interval(randomDouble() * 1000);
IllegalArgumentException ex = expectThrows(IllegalArgumentException.class, () -> { factory.extendedBounds(Double.NaN, 1.0); });
assertThat(ex.getMessage(), startsWith("minBound must be finite, got: "));
ex = expectThrows(IllegalArgumentException.class, () -> { factory.extendedBounds(Double.POSITIVE_INFINITY, 1.0); });
assertThat(ex.getMessage(), startsWith("minBound must be finite, got: "));
ex = expectThrows(IllegalArgumentException.class, () -> { factory.extendedBounds(Double.NEGATIVE_INFINITY, 1.0); });
assertThat(ex.getMessage(), startsWith("minBound must be finite, got: "));
ex = expectThrows(IllegalArgumentException.class, () -> { factory.extendedBounds(0.0, Double.NaN); });
assertThat(ex.getMessage(), startsWith("maxBound must be finite, got: "));
ex = expectThrows(IllegalArgumentException.class, () -> { factory.extendedBounds(0.0, Double.POSITIVE_INFINITY); });
assertThat(ex.getMessage(), startsWith("maxBound must be finite, got: "));
ex = expectThrows(IllegalArgumentException.class, () -> { factory.extendedBounds(0.0, Double.NEGATIVE_INFINITY); });
assertThat(ex.getMessage(), startsWith("maxBound must be finite, got: "));
ex = expectThrows(IllegalArgumentException.class, () -> { factory.extendedBounds(0.5, 0.4); });
assertThat(ex.getMessage(), equalTo("maxBound [0.4] must be greater than minBound [0.5]"));
}
}