You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@iceberg.apache.org by bl...@apache.org on 2022/11/06 21:15:47 UTC
[iceberg] branch master updated: API: Fix TestAggregateBinding (#6065)
This is an automated email from the ASF dual-hosted git repository.
blue pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/iceberg.git
The following commit(s) were added to refs/heads/master by this push:
new bcd15188aa API: Fix TestAggregateBinding (#6065)
bcd15188aa is described below
commit bcd15188aa9f73609b986c033df5b03eeaa07b58
Author: Huaxin Gao <hu...@apple.com>
AuthorDate: Sun Nov 6 13:15:42 2022 -0800
API: Fix TestAggregateBinding (#6065)
---
.../apache/iceberg/expressions/Expressions.java | 8 ++--
.../iceberg/expressions/TestAggregateBinding.java | 50 ++++++----------------
2 files changed, 18 insertions(+), 40 deletions(-)
diff --git a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
index 171da823cc..f21a770596 100644
--- a/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
+++ b/api/src/main/java/org/apache/iceberg/expressions/Expressions.java
@@ -309,19 +309,19 @@ public class Expressions {
return new UnboundTransform<>(ref(name), transform);
}
- public static UnboundAggregate<String> count(String name) {
+ public static <T> UnboundAggregate<T> count(String name) {
return new UnboundAggregate<>(Operation.COUNT, ref(name));
}
- public static UnboundAggregate<String> countStar() {
+ public static <T> UnboundAggregate<T> countStar() {
return new UnboundAggregate<>(Operation.COUNT_STAR, null);
}
- public static UnboundAggregate<String> max(String name) {
+ public static <T> UnboundAggregate<T> max(String name) {
return new UnboundAggregate<>(Operation.MAX, ref(name));
}
- public static UnboundAggregate<String> min(String name) {
+ public static <T> UnboundAggregate<T> min(String name) {
return new UnboundAggregate<>(Operation.MIN, ref(name));
}
}
diff --git a/api/src/test/java/org/apache/iceberg/expressions/TestAggregateBinding.java b/api/src/test/java/org/apache/iceberg/expressions/TestAggregateBinding.java
index 7bbe8ad7da..859ee89a9f 100644
--- a/api/src/test/java/org/apache/iceberg/expressions/TestAggregateBinding.java
+++ b/api/src/test/java/org/apache/iceberg/expressions/TestAggregateBinding.java
@@ -18,9 +18,9 @@
*/
package org.apache.iceberg.expressions;
-import java.util.Arrays;
import java.util.List;
import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableList;
import org.apache.iceberg.types.Types;
import org.apache.iceberg.types.Types.StructType;
import org.assertj.core.api.Assertions;
@@ -28,42 +28,26 @@ import org.junit.Assert;
import org.junit.Test;
public class TestAggregateBinding {
- private static final List<Expression.Operation> AGGREGATES =
- Arrays.asList(Expression.Operation.COUNT, Expression.Operation.MAX, Expression.Operation.MIN);
+ private static final List<UnboundAggregate<Integer>> list =
+ ImmutableList.of(Expressions.count("x"), Expressions.max("x"), Expressions.min("x"));
private static final StructType struct =
StructType.of(Types.NestedField.required(10, "x", Types.IntegerType.get()));
@Test
public void testAggregateBinding() {
- for (Expression.Operation op : AGGREGATES) {
- UnboundAggregate unbound = null;
- switch (op) {
- case COUNT:
- unbound = Expressions.count("x");
- break;
- case MAX:
- unbound = Expressions.max("x");
- break;
- case MIN:
- unbound = Expressions.min("x");
- break;
- default:
- throw new UnsupportedOperationException("Invalid aggregate: " + op);
- }
-
+ for (UnboundAggregate<Integer> unbound : list) {
Expression expr = unbound.bind(struct, true);
- BoundAggregate bound = assertAndUnwrapAggregate(expr);
-
+ BoundAggregate<Integer, ?> bound = assertAndUnwrapAggregate(expr);
Assert.assertEquals("Should reference correct field ID", 10, bound.ref().fieldId());
- Assert.assertEquals("Should not change the comparison operation", op, bound.op());
+ Assert.assertEquals("Should not change the comparison operation", unbound.op(), bound.op());
}
}
@Test
public void testCountStarBinding() {
- UnboundAggregate unbound = Expressions.countStar();
+ UnboundAggregate<?> unbound = Expressions.countStar();
Expression expr = unbound.bind(null, false);
- BoundAggregate bound = assertAndUnwrapAggregate(expr);
+ BoundAggregate<?, Long> bound = assertAndUnwrapAggregate(expr);
Assert.assertEquals(
"Should not change the comparison operation", Expression.Operation.COUNT_STAR, bound.op());
@@ -81,7 +65,7 @@ public class TestAggregateBinding {
public void testCaseInsensitiveReference() {
Expression expr = Expressions.max("X");
Expression boundExpr = Binder.bind(struct, expr, false);
- BoundAggregate bound = assertAndUnwrapAggregate(boundExpr);
+ BoundAggregate<Integer, Integer> bound = assertAndUnwrapAggregate(boundExpr);
Assert.assertEquals("Should reference correct field ID", 10, bound.ref().fieldId());
Assert.assertEquals(
"Should not change the comparison operation", Expression.Operation.MAX, bound.op());
@@ -97,20 +81,14 @@ public class TestAggregateBinding {
@Test
public void testMissingField() {
- UnboundAggregate unbound = Expressions.count("missing");
- try {
- unbound.bind(struct, false);
- Assert.fail("Binding a missing field should fail");
- } catch (ValidationException e) {
- Assert.assertTrue(
- "Validation should complain about missing field",
- e.getMessage().contains("Cannot find field 'missing' in struct:"));
- }
+ UnboundAggregate<?> unbound = Expressions.count("missing");
+ Assertions.assertThatThrownBy(() -> unbound.bind(struct, false))
+ .isInstanceOf(ValidationException.class)
+ .hasMessageContaining("Cannot find field 'missing' in struct:");
}
private static <T, C> BoundAggregate<T, C> assertAndUnwrapAggregate(Expression expr) {
- Assert.assertTrue(
- "Expression should be a bound aggregate: " + expr, expr instanceof BoundAggregate);
+ Assertions.assertThat(expr).isInstanceOf(BoundAggregate.class);
return (BoundAggregate<T, C>) expr;
}
}