You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "nastra (via GitHub)" <gi...@apache.org> on 2026/07/30 08:57:41 UTC
[PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
nastra opened a new pull request, #17430:
URL: https://github.com/apache/iceberg/pull/17430
This makes the three test classes extendible so that we can introduce separate test classes that use a content stats evaluator but run through the same set of tests
**AI Disclosure**
- Model: Claude Opus 5
- Platform/Tool: Cursor
- Human Oversight: fully reviewed
- Prompt Summary: Refactor TestInclusiveMetricsEvaluator, TestInclusiveMetricsEvaluatorWithExtract, and TestInclusiveMetricsEvaluatorWithTransforms into extendable harnesses by hoisting inline file fixtures into protected factory methods and routing evaluation through a protected shouldRead hook, so subclasses can run the same expression suite against a different file statistics representation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on PR #17430:
URL: https://github.com/apache/iceberg/pull/17430#issuecomment-5297027664
I'll go ahead and close this one since we got the changes merged as part of https://github.com/apache/iceberg/pull/17413. Thanks everyone for reviewing
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #17430:
URL: https://github.com/apache/iceberg/pull/17430#discussion_r3694981686
##########
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluator.java:
##########
@@ -393,789 +546,521 @@ public void testZeroRecordFile() {
};
for (Expression expr : exprs) {
- boolean shouldRead = new InclusiveMetricsEvaluator(SCHEMA, expr).eval(empty);
+ boolean shouldRead = shouldRead(SCHEMA, expr, emptyFile());
assertThat(shouldRead).as("Should never read 0-record file: " + expr).isFalse();
}
}
@Test
public void testNot() {
// this test case must use a real predicate, not alwaysTrue(), or binding will simplify it out
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(lessThan("id", INT_MIN_VALUE - 25))).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, not(lessThan("id", INT_MIN_VALUE - 25)), file());
assertThat(shouldRead).as("Should read: not(false)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(greaterThan("id", INT_MIN_VALUE - 25)))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(greaterThan("id", INT_MIN_VALUE - 25)), file());
assertThat(shouldRead).as("Should skip: not(true)").isFalse();
}
@Test
public void testAnd() {
// this test case must use a real predicate, not alwaysTrue(), or binding will simplify it out
boolean shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- and(
- lessThan("id", INT_MIN_VALUE - 25),
- greaterThanOrEqual("id", INT_MIN_VALUE - 30)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ and(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MIN_VALUE - 30)),
+ file());
assertThat(shouldRead).as("Should skip: and(false, true)").isFalse();
shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- and(
- lessThan("id", INT_MIN_VALUE - 25),
- greaterThanOrEqual("id", INT_MAX_VALUE + 1)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ and(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MAX_VALUE + 1)),
+ file());
assertThat(shouldRead).as("Should skip: and(false, false)").isFalse();
shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- and(greaterThan("id", INT_MIN_VALUE - 25), lessThanOrEqual("id", INT_MIN_VALUE)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ and(greaterThan("id", INT_MIN_VALUE - 25), lessThanOrEqual("id", INT_MIN_VALUE)),
+ file());
assertThat(shouldRead).as("Should read: and(true, true)").isTrue();
}
@Test
public void testOr() {
// this test case must use a real predicate, not alwaysTrue(), or binding will simplify it out
boolean shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- or(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MAX_VALUE + 1)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ or(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MAX_VALUE + 1)),
+ file());
assertThat(shouldRead).as("Should skip: or(false, false)").isFalse();
shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- or(
- lessThan("id", INT_MIN_VALUE - 25),
- greaterThanOrEqual("id", INT_MAX_VALUE - 19)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ or(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MAX_VALUE - 19)),
+ file());
assertThat(shouldRead).as("Should read: or(false, true)").isTrue();
}
@Test
public void testIntegerLt() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThan("id", INT_MIN_VALUE - 25)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, lessThan("id", INT_MIN_VALUE - 25), file());
assertThat(shouldRead).as("Should not read: id range below lower bound (5 < 30)").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, lessThan("id", INT_MIN_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThan("id", INT_MIN_VALUE), file());
assertThat(shouldRead)
.as("Should not read: id range below lower bound (30 is not < 30)")
.isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThan("id", INT_MIN_VALUE + 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThan("id", INT_MIN_VALUE + 1), file());
assertThat(shouldRead).as("Should read: one possible id").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, lessThan("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThan("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: many possible ids").isTrue();
}
@Test
public void testIntegerLtEq() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE - 25)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE - 25), file());
assertThat(shouldRead).as("Should not read: id range below lower bound (5 < 30)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE - 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should not read: id range below lower bound (29 < 30)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: one possible id").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThanOrEqual("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThanOrEqual("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: many possible ids").isTrue();
}
@Test
public void testIntegerGt() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThan("id", INT_MAX_VALUE + 6)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, greaterThan("id", INT_MAX_VALUE + 6), file());
assertThat(shouldRead).as("Should not read: id range above upper bound (85 < 79)").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, greaterThan("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThan("id", INT_MAX_VALUE), file());
assertThat(shouldRead)
.as("Should not read: id range above upper bound (79 is not > 79)")
.isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThan("id", INT_MAX_VALUE - 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThan("id", INT_MAX_VALUE - 1), file());
assertThat(shouldRead).as("Should read: one possible id").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThan("id", INT_MAX_VALUE - 4)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThan("id", INT_MAX_VALUE - 4), file());
assertThat(shouldRead).as("Should read: many possible ids").isTrue();
}
@Test
public void testIntegerGtEq() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE + 6))
- .eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE + 6), file());
assertThat(shouldRead).as("Should not read: id range above upper bound (85 < 79)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE + 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should not read: id range above upper bound (80 > 79)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: one possible id").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE - 4))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE - 4), file());
assertThat(shouldRead).as("Should read: many possible ids").isTrue();
}
@Test
public void testIntegerEq() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MIN_VALUE - 25)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, equal("id", INT_MIN_VALUE - 25), file());
assertThat(shouldRead).as("Should not read: id below lower bound").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MIN_VALUE - 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should not read: id below lower bound").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MIN_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to lower bound").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MAX_VALUE - 4)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MAX_VALUE - 4), file());
assertThat(shouldRead).as("Should read: id between lower and upper bounds").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to upper bound").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MAX_VALUE + 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should not read: id above upper bound").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MAX_VALUE + 6)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MAX_VALUE + 6), file());
assertThat(shouldRead).as("Should not read: id above upper bound").isFalse();
}
@Test
public void testIntegerNotEq() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MIN_VALUE - 25)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MIN_VALUE - 25), file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MIN_VALUE - 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MIN_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MAX_VALUE - 4)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MAX_VALUE - 4), file());
assertThat(shouldRead).as("Should read: id between lower and upper bounds").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MAX_VALUE + 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MAX_VALUE + 6)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MAX_VALUE + 6), file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
}
@Test
public void testIntegerNotEqRewritten() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MIN_VALUE - 25))).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MIN_VALUE - 25)), file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MIN_VALUE - 1))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MIN_VALUE - 1)), file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MIN_VALUE))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MIN_VALUE)), file());
assertThat(shouldRead).as("Should read: id equal to lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MAX_VALUE - 4))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MAX_VALUE - 4)), file());
assertThat(shouldRead).as("Should read: id between lower and upper bounds").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MAX_VALUE))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MAX_VALUE)), file());
assertThat(shouldRead).as("Should read: id equal to upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MAX_VALUE + 1))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MAX_VALUE + 1)), file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MAX_VALUE + 6))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MAX_VALUE + 6)), file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
}
@Test
public void testCaseInsensitiveIntegerNotEqRewritten() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MIN_VALUE - 25)), false)
- .eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MIN_VALUE - 25)), false, file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MIN_VALUE - 1)), false)
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MIN_VALUE - 1)), false, file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MIN_VALUE)), false).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MIN_VALUE)), false, file());
assertThat(shouldRead).as("Should read: id equal to lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MAX_VALUE - 4)), false)
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MAX_VALUE - 4)), false, file());
assertThat(shouldRead).as("Should read: id between lower and upper bounds").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MAX_VALUE)), false).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MAX_VALUE)), false, file());
assertThat(shouldRead).as("Should read: id equal to upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MAX_VALUE + 1)), false)
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MAX_VALUE + 1)), false, file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MAX_VALUE + 6)), false)
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MAX_VALUE + 6)), false, file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
}
@Test
public void testCaseSensitiveIntegerNotEqRewritten() {
- assertThatThrownBy(
- () -> new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", 5)), true).eval(FILE))
+ assertThatThrownBy(() -> shouldRead(SCHEMA, not(equal("ID", 5)), true, file()))
.isInstanceOf(ValidationException.class)
.hasMessageContaining("Cannot find field 'ID'");
}
@Test
public void testStringStartsWith() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "a"), true).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, startsWith("required", "a"), true, file());
assertThat(shouldRead).as("Should read: no stats").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "a"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "a"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "aa"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "aa"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "aaa"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "aaa"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "1s"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "1s"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "1str1x"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "1str1x"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "ff"), true).eval(FILE_4);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "ff"), true, file4());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "aB"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "aB"), true, file2());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "dWX"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "dWX"), true, file2());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "5"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "5"), true, file3());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "3str3x"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "3str3x"), true, file3());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("some_empty", "房东整租霍"), true).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, startsWith("some_empty", "房东整租霍"), true, file());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("all_nulls", ""), true).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, startsWith("all_nulls", ""), true, file());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
String aboveMax = UnicodeUtil.truncateStringMax(Literal.of("イロハニホヘト"), 4).value().toString();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", aboveMax), true).eval(FILE_4);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", aboveMax), true, file4());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
}
@Test
public void testStringNotStartsWith() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "a"), true).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, notStartsWith("required", "a"), true, file());
assertThat(shouldRead).as("Should read: no stats").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "a"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "a"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "aa"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "aa"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "aaa"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "aaa"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "1s"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "1s"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "1str1x"), true)
- .eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "1str1x"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "ff"), true).eval(FILE_4);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "ff"), true, file4());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "aB"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "aB"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "dWX"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "dWX"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "5"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "5"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "3str3x"), true)
- .eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "3str3x"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
String aboveMax = UnicodeUtil.truncateStringMax(Literal.of("イロハニホヘト"), 4).value().toString();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", aboveMax), true)
- .eval(FILE_4);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", aboveMax), true, file4());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "abc"), true).eval(FILE_5);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "abc"), true, file5());
assertThat(shouldRead).as("Should not read: all strings start with prefix").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "abcd"), true).eval(FILE_5);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "abcd"), true, file5());
assertThat(shouldRead).as("Should not read: lower shorter than prefix, cannot match").isTrue();
}
@Test
public void testIntegerIn() {
boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MIN_VALUE - 25, INT_MIN_VALUE - 24))
- .eval(FILE);
+ shouldRead(SCHEMA, in("id", INT_MIN_VALUE - 25, INT_MIN_VALUE - 24), file());
assertThat(shouldRead).as("Should not read: id below lower bound (5 < 30, 6 < 30)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MIN_VALUE - 2, INT_MIN_VALUE - 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MIN_VALUE - 2, INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should not read: id below lower bound (28 < 30, 29 < 30)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MIN_VALUE - 1, INT_MIN_VALUE))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MIN_VALUE - 1, INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to lower bound (30 == 30)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MAX_VALUE - 4, INT_MAX_VALUE - 3))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MAX_VALUE - 4, INT_MAX_VALUE - 3), file());
assertThat(shouldRead)
.as("Should read: id between lower and upper bounds (30 < 75 < 79, 30 < 76 < 79)")
.isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MAX_VALUE, INT_MAX_VALUE + 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MAX_VALUE, INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should read: id equal to upper bound (79 == 79)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MAX_VALUE + 1, INT_MAX_VALUE + 2))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MAX_VALUE + 1, INT_MAX_VALUE + 2), file());
assertThat(shouldRead).as("Should not read: id above upper bound (80 > 79, 81 > 79)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MAX_VALUE + 6, INT_MAX_VALUE + 7))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MAX_VALUE + 6, INT_MAX_VALUE + 7), file());
assertThat(shouldRead).as("Should not read: id above upper bound (85 > 79, 86 > 79)").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, in("all_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("all_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should skip: in on all nulls column").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, in("some_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("some_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: in on some nulls column").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, in("no_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("no_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: in on no nulls column").isTrue();
// should read as the number of elements in the in expression is too big
List<Integer> ids = Lists.newArrayListWithExpectedSize(400);
for (int id = -400; id <= 0; id++) {
ids.add(id);
}
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, in("id", ids)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", ids), file());
assertThat(shouldRead).as("Should read: large in expression").isTrue();
}
@Test
public void testIntegerNotIn() {
boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MIN_VALUE - 25, INT_MIN_VALUE - 24))
- .eval(FILE);
+ shouldRead(SCHEMA, notIn("id", INT_MIN_VALUE - 25, INT_MIN_VALUE - 24), file());
assertThat(shouldRead).as("Should read: id below lower bound (5 < 30, 6 < 30)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MIN_VALUE - 2, INT_MIN_VALUE - 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MIN_VALUE - 2, INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should read: id below lower bound (28 < 30, 29 < 30)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MIN_VALUE - 1, INT_MIN_VALUE))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MIN_VALUE - 1, INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to lower bound (30 == 30)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MAX_VALUE - 4, INT_MAX_VALUE - 3))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MAX_VALUE - 4, INT_MAX_VALUE - 3), file());
assertThat(shouldRead)
.as("Should read: id between lower and upper bounds (30 < 75 < 79, 30 < 76 < 79)")
.isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MAX_VALUE, INT_MAX_VALUE + 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MAX_VALUE, INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should read: id equal to upper bound (79 == 79)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MAX_VALUE + 1, INT_MAX_VALUE + 2))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MAX_VALUE + 1, INT_MAX_VALUE + 2), file());
assertThat(shouldRead).as("Should read: id above upper bound (80 > 79, 81 > 79)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MAX_VALUE + 6, INT_MAX_VALUE + 7))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MAX_VALUE + 6, INT_MAX_VALUE + 7), file());
assertThat(shouldRead).as("Should read: id above upper bound (85 > 79, 86 > 79)").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, notIn("all_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("all_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: notIn on all nulls column").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("some_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("some_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: notIn on some nulls column").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, notIn("no_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("no_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: notIn on no nulls column").isTrue();
}
@Test
public void testIsNullInNestedStruct() {
// read required_address and its nested fields
- boolean shouldRead =
- new InclusiveMetricsEvaluator(NESTED_SCHEMA, isNull("required_address")).eval(FILE_6);
+ boolean shouldRead = shouldRead(NESTED_SCHEMA, isNull("required_address"), file6());
assertThat(shouldRead).as("Should not read: required_address is required").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(NESTED_SCHEMA, isNull("required_address.required_street1"))
- .eval(FILE_6);
+ shouldRead = shouldRead(NESTED_SCHEMA, isNull("required_address.required_street1"), file6());
assertThat(shouldRead)
.as("Should not read: required_address.required_street1 is required")
.isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(NESTED_SCHEMA, isNull("required_address.optional_street1"))
- .eval(FILE_6);
+ shouldRead = shouldRead(NESTED_SCHEMA, isNull("required_address.optional_street1"), file6());
assertThat(shouldRead)
.as("Should read: required_address.optional_street1 is optional")
.isTrue();
// read optional_address and its nested fields
- shouldRead =
- new InclusiveMetricsEvaluator(NESTED_SCHEMA, isNull("optional_address")).eval(FILE_6);
+ shouldRead = shouldRead(NESTED_SCHEMA, isNull("optional_address"), file6());
Review Comment:
it was building on top of these changes yes, but I've rebased the PR now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "pvary (via GitHub)" <gi...@apache.org>.
pvary commented on code in PR #17430:
URL: https://github.com/apache/iceberg/pull/17430#discussion_r3759312642
##########
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluator.java:
##########
@@ -57,8 +57,8 @@
import org.apache.iceberg.util.UnicodeUtil;
import org.junit.jupiter.api.Test;
-public class TestInclusiveMetricsEvaluator {
- private static final Schema SCHEMA =
+public class TestInclusiveMetricsEvaluator<F> {
Review Comment:
Are we mixing 2 classes here?
- Base class where the abstract `protected F file()` is defined
- Implementation class where the `protected static final DataFile FILE` is defined
##########
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluator.java:
##########
@@ -57,8 +57,8 @@
import org.apache.iceberg.util.UnicodeUtil;
import org.junit.jupiter.api.Test;
-public class TestInclusiveMetricsEvaluator {
- private static final Schema SCHEMA =
+public class TestInclusiveMetricsEvaluator<F> {
Review Comment:
Are we mixing 2 classes here?
- Base class where the abstract `protected F file()` is defined
- Implementation class where the `protected static final DataFile FILE` is defined and the `protected F file()` is implemented using that?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #17430:
URL: https://github.com/apache/iceberg/pull/17430#discussion_r3694976261
##########
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluator.java:
##########
@@ -210,150 +212,303 @@ public class TestInclusiveMetricsEvaluator {
Row.of(),
10,
// any value counts, including nulls
- ImmutableMap.of(100, 5L, 101, 5L, 102, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(102, 5L, 103, 5L, 104, 5L, 105, 5L),
// null value counts
- ImmutableMap.of(100, 0L, 101, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(103, 5L, 104, 5L, 105, 5L),
// nan value counts
null,
// lower bounds
null,
// upper bounds
null);
+ private static final DataFile MISSING_STATS = new TestDataFile("file.parquet", Row.of(), 50);
+
+ private static final DataFile EMPTY_FILE = new TestDataFile("file.parquet", Row.of(), 0);
+
+ private static final DataFile RANGE_OF_VALUES =
+ new TestDataFile(
+ "range_of_values.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(3, 10L),
+ ImmutableMap.of(3, 0L),
+ null,
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "aaa")),
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "zzz")));
+
+ private static final DataFile SINGLE_VALUE_FILE =
+ new TestDataFile(
+ "single_value.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(3, 10L),
+ ImmutableMap.of(3, 0L),
+ null,
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "abc")),
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "abc")));
+
+ // some_empty is optional because a required column cannot contain nulls
+ private static final DataFile SINGLE_VALUE_WITH_NULLS =
+ new TestDataFile(
+ "single_value_nulls.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(14, 10L),
+ ImmutableMap.of(14, 2L),
+ null,
+ ImmutableMap.of(14, toByteBuffer(StringType.get(), "abc")),
+ ImmutableMap.of(14, toByteBuffer(StringType.get(), "abc")));
+
+ private static final DataFile SINGLE_VALUE_WITH_NAN =
+ new TestDataFile(
+ "single_value_nan.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(9, 10L),
+ ImmutableMap.of(9, 0L),
+ ImmutableMap.of(9, 2L),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), 5.0F)),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), 5.0F)));
+
+ private static final DataFile SINGLE_VALUE_NAN_BOUNDS =
+ new TestDataFile(
+ "single_value_nan_bounds.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(9, 10L),
+ ImmutableMap.of(9, 0L),
+ ImmutableMap.of(9, 0L),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), Float.NaN)),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), Float.NaN)));
+
+ private static final Map<Integer, ByteBuffer> FLOAT_BOUND =
+ ImmutableMap.of(1, toByteBuffer(Types.FloatType.get(), 1.0f));
+
+ private static final DataFile SINGLE_FLOAT_VALUE_FILE =
+ new TestDataFile(
+ "single_value_file.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(1, 10L),
+ ImmutableMap.of(1, 0L),
+ ImmutableMap.of(1, 0L),
+ FLOAT_BOUND,
+ FLOAT_BOUND);
+
+ private static final DataFile SINGLE_FLOAT_VALUE_FILE_WITH_NAN =
+ new TestDataFile(
+ "single_value_file.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(1, 10L),
+ ImmutableMap.of(1, 0L),
+ ImmutableMap.of(1, 1L), // contains a NaN value
+ FLOAT_BOUND,
+ FLOAT_BOUND);
+
+ protected boolean shouldRead(Schema schema, Expression expr, boolean caseSensitive, F testFile) {
+ return new InclusiveMetricsEvaluator(schema, expr, caseSensitive).eval((DataFile) testFile);
+ }
+
+ protected boolean shouldRead(Schema schema, Expression expr, F testFile) {
+ return shouldRead(schema, expr, true, testFile);
+ }
+
+ protected F file() {
Review Comment:
I guess we could do that but we'd have to go through the TrackedFileAdapter to convert from TrackedFile to DataFile. My idea was to create a TrackedFile directly in the subclass, like in https://github.com/apache/iceberg/pull/17413/changes#diff-c00cac79784b4d5c0d5c123c998f85bd6267f6baef20eaf0a1b2373c17ca1ac5R53
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #17430:
URL: https://github.com/apache/iceberg/pull/17430#discussion_r3692590716
##########
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluator.java:
##########
@@ -210,150 +212,303 @@ public class TestInclusiveMetricsEvaluator {
Row.of(),
10,
// any value counts, including nulls
- ImmutableMap.of(100, 5L, 101, 5L, 102, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(102, 5L, 103, 5L, 104, 5L, 105, 5L),
// null value counts
- ImmutableMap.of(100, 0L, 101, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(103, 5L, 104, 5L, 105, 5L),
// nan value counts
null,
// lower bounds
null,
// upper bounds
null);
+ private static final DataFile MISSING_STATS = new TestDataFile("file.parquet", Row.of(), 50);
+
+ private static final DataFile EMPTY_FILE = new TestDataFile("file.parquet", Row.of(), 0);
+
+ private static final DataFile RANGE_OF_VALUES =
+ new TestDataFile(
+ "range_of_values.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(3, 10L),
+ ImmutableMap.of(3, 0L),
+ null,
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "aaa")),
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "zzz")));
+
+ private static final DataFile SINGLE_VALUE_FILE =
+ new TestDataFile(
+ "single_value.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(3, 10L),
+ ImmutableMap.of(3, 0L),
+ null,
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "abc")),
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "abc")));
+
+ // some_empty is optional because a required column cannot contain nulls
+ private static final DataFile SINGLE_VALUE_WITH_NULLS =
+ new TestDataFile(
+ "single_value_nulls.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(14, 10L),
+ ImmutableMap.of(14, 2L),
+ null,
+ ImmutableMap.of(14, toByteBuffer(StringType.get(), "abc")),
+ ImmutableMap.of(14, toByteBuffer(StringType.get(), "abc")));
+
+ private static final DataFile SINGLE_VALUE_WITH_NAN =
+ new TestDataFile(
+ "single_value_nan.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(9, 10L),
+ ImmutableMap.of(9, 0L),
+ ImmutableMap.of(9, 2L),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), 5.0F)),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), 5.0F)));
+
+ private static final DataFile SINGLE_VALUE_NAN_BOUNDS =
+ new TestDataFile(
+ "single_value_nan_bounds.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(9, 10L),
+ ImmutableMap.of(9, 0L),
+ ImmutableMap.of(9, 0L),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), Float.NaN)),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), Float.NaN)));
+
+ private static final Map<Integer, ByteBuffer> FLOAT_BOUND =
+ ImmutableMap.of(1, toByteBuffer(Types.FloatType.get(), 1.0f));
+
+ private static final DataFile SINGLE_FLOAT_VALUE_FILE =
+ new TestDataFile(
+ "single_value_file.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(1, 10L),
+ ImmutableMap.of(1, 0L),
+ ImmutableMap.of(1, 0L),
+ FLOAT_BOUND,
+ FLOAT_BOUND);
+
+ private static final DataFile SINGLE_FLOAT_VALUE_FILE_WITH_NAN =
+ new TestDataFile(
+ "single_value_file.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(1, 10L),
+ ImmutableMap.of(1, 0L),
+ ImmutableMap.of(1, 1L), // contains a NaN value
+ FLOAT_BOUND,
+ FLOAT_BOUND);
+
+ protected boolean shouldRead(Schema schema, Expression expr, boolean caseSensitive, F testFile) {
+ return new InclusiveMetricsEvaluator(schema, expr, caseSensitive).eval((DataFile) testFile);
+ }
+
+ protected boolean shouldRead(Schema schema, Expression expr, F testFile) {
+ return shouldRead(schema, expr, true, testFile);
+ }
+
+ protected F file() {
Review Comment:
Can't we just make the data files protected? I feel like the indirection through these accessors is more confusing than just exposing the test files to children.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #17430:
URL: https://github.com/apache/iceberg/pull/17430#discussion_r3692567049
##########
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluator.java:
##########
@@ -210,150 +212,303 @@ public class TestInclusiveMetricsEvaluator {
Row.of(),
10,
// any value counts, including nulls
- ImmutableMap.of(100, 5L, 101, 5L, 102, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(102, 5L, 103, 5L, 104, 5L, 105, 5L),
// null value counts
- ImmutableMap.of(100, 0L, 101, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(103, 5L, 104, 5L, 105, 5L),
Review Comment:
Looks like these changes are from the other PR #17412?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra closed pull request #17430: API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable
URL: https://github.com/apache/iceberg/pull/17430
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #17430:
URL: https://github.com/apache/iceberg/pull/17430#discussion_r3684369097
##########
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluator.java:
##########
@@ -393,789 +546,521 @@ public void testZeroRecordFile() {
};
for (Expression expr : exprs) {
- boolean shouldRead = new InclusiveMetricsEvaluator(SCHEMA, expr).eval(empty);
+ boolean shouldRead = shouldRead(SCHEMA, expr, emptyFile());
assertThat(shouldRead).as("Should never read 0-record file: " + expr).isFalse();
}
}
@Test
public void testNot() {
// this test case must use a real predicate, not alwaysTrue(), or binding will simplify it out
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(lessThan("id", INT_MIN_VALUE - 25))).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, not(lessThan("id", INT_MIN_VALUE - 25)), file());
assertThat(shouldRead).as("Should read: not(false)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(greaterThan("id", INT_MIN_VALUE - 25)))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(greaterThan("id", INT_MIN_VALUE - 25)), file());
assertThat(shouldRead).as("Should skip: not(true)").isFalse();
}
@Test
public void testAnd() {
// this test case must use a real predicate, not alwaysTrue(), or binding will simplify it out
boolean shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- and(
- lessThan("id", INT_MIN_VALUE - 25),
- greaterThanOrEqual("id", INT_MIN_VALUE - 30)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ and(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MIN_VALUE - 30)),
+ file());
assertThat(shouldRead).as("Should skip: and(false, true)").isFalse();
shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- and(
- lessThan("id", INT_MIN_VALUE - 25),
- greaterThanOrEqual("id", INT_MAX_VALUE + 1)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ and(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MAX_VALUE + 1)),
+ file());
assertThat(shouldRead).as("Should skip: and(false, false)").isFalse();
shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- and(greaterThan("id", INT_MIN_VALUE - 25), lessThanOrEqual("id", INT_MIN_VALUE)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ and(greaterThan("id", INT_MIN_VALUE - 25), lessThanOrEqual("id", INT_MIN_VALUE)),
+ file());
assertThat(shouldRead).as("Should read: and(true, true)").isTrue();
}
@Test
public void testOr() {
// this test case must use a real predicate, not alwaysTrue(), or binding will simplify it out
boolean shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- or(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MAX_VALUE + 1)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ or(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MAX_VALUE + 1)),
+ file());
assertThat(shouldRead).as("Should skip: or(false, false)").isFalse();
shouldRead =
- new InclusiveMetricsEvaluator(
- SCHEMA,
- or(
- lessThan("id", INT_MIN_VALUE - 25),
- greaterThanOrEqual("id", INT_MAX_VALUE - 19)))
- .eval(FILE);
+ shouldRead(
+ SCHEMA,
+ or(lessThan("id", INT_MIN_VALUE - 25), greaterThanOrEqual("id", INT_MAX_VALUE - 19)),
+ file());
assertThat(shouldRead).as("Should read: or(false, true)").isTrue();
}
@Test
public void testIntegerLt() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThan("id", INT_MIN_VALUE - 25)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, lessThan("id", INT_MIN_VALUE - 25), file());
assertThat(shouldRead).as("Should not read: id range below lower bound (5 < 30)").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, lessThan("id", INT_MIN_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThan("id", INT_MIN_VALUE), file());
assertThat(shouldRead)
.as("Should not read: id range below lower bound (30 is not < 30)")
.isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThan("id", INT_MIN_VALUE + 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThan("id", INT_MIN_VALUE + 1), file());
assertThat(shouldRead).as("Should read: one possible id").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, lessThan("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThan("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: many possible ids").isTrue();
}
@Test
public void testIntegerLtEq() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE - 25)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE - 25), file());
assertThat(shouldRead).as("Should not read: id range below lower bound (5 < 30)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE - 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should not read: id range below lower bound (29 < 30)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThanOrEqual("id", INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: one possible id").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, lessThanOrEqual("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, lessThanOrEqual("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: many possible ids").isTrue();
}
@Test
public void testIntegerGt() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThan("id", INT_MAX_VALUE + 6)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, greaterThan("id", INT_MAX_VALUE + 6), file());
assertThat(shouldRead).as("Should not read: id range above upper bound (85 < 79)").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, greaterThan("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThan("id", INT_MAX_VALUE), file());
assertThat(shouldRead)
.as("Should not read: id range above upper bound (79 is not > 79)")
.isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThan("id", INT_MAX_VALUE - 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThan("id", INT_MAX_VALUE - 1), file());
assertThat(shouldRead).as("Should read: one possible id").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThan("id", INT_MAX_VALUE - 4)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThan("id", INT_MAX_VALUE - 4), file());
assertThat(shouldRead).as("Should read: many possible ids").isTrue();
}
@Test
public void testIntegerGtEq() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE + 6))
- .eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE + 6), file());
assertThat(shouldRead).as("Should not read: id range above upper bound (85 < 79)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE + 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should not read: id range above upper bound (80 > 79)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: one possible id").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE - 4))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, greaterThanOrEqual("id", INT_MAX_VALUE - 4), file());
assertThat(shouldRead).as("Should read: many possible ids").isTrue();
}
@Test
public void testIntegerEq() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MIN_VALUE - 25)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, equal("id", INT_MIN_VALUE - 25), file());
assertThat(shouldRead).as("Should not read: id below lower bound").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MIN_VALUE - 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should not read: id below lower bound").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MIN_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to lower bound").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MAX_VALUE - 4)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MAX_VALUE - 4), file());
assertThat(shouldRead).as("Should read: id between lower and upper bounds").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to upper bound").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MAX_VALUE + 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should not read: id above upper bound").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, equal("id", INT_MAX_VALUE + 6)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, equal("id", INT_MAX_VALUE + 6), file());
assertThat(shouldRead).as("Should not read: id above upper bound").isFalse();
}
@Test
public void testIntegerNotEq() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MIN_VALUE - 25)).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MIN_VALUE - 25), file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MIN_VALUE - 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MIN_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MAX_VALUE - 4)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MAX_VALUE - 4), file());
assertThat(shouldRead).as("Should read: id between lower and upper bounds").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MAX_VALUE)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MAX_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MAX_VALUE + 1)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notEqual("id", INT_MAX_VALUE + 6)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notEqual("id", INT_MAX_VALUE + 6), file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
}
@Test
public void testIntegerNotEqRewritten() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MIN_VALUE - 25))).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MIN_VALUE - 25)), file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MIN_VALUE - 1))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MIN_VALUE - 1)), file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MIN_VALUE))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MIN_VALUE)), file());
assertThat(shouldRead).as("Should read: id equal to lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MAX_VALUE - 4))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MAX_VALUE - 4)), file());
assertThat(shouldRead).as("Should read: id between lower and upper bounds").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MAX_VALUE))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MAX_VALUE)), file());
assertThat(shouldRead).as("Should read: id equal to upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MAX_VALUE + 1))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MAX_VALUE + 1)), file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("id", INT_MAX_VALUE + 6))).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("id", INT_MAX_VALUE + 6)), file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
}
@Test
public void testCaseInsensitiveIntegerNotEqRewritten() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MIN_VALUE - 25)), false)
- .eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MIN_VALUE - 25)), false, file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MIN_VALUE - 1)), false)
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MIN_VALUE - 1)), false, file());
assertThat(shouldRead).as("Should read: id below lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MIN_VALUE)), false).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MIN_VALUE)), false, file());
assertThat(shouldRead).as("Should read: id equal to lower bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MAX_VALUE - 4)), false)
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MAX_VALUE - 4)), false, file());
assertThat(shouldRead).as("Should read: id between lower and upper bounds").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MAX_VALUE)), false).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MAX_VALUE)), false, file());
assertThat(shouldRead).as("Should read: id equal to upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MAX_VALUE + 1)), false)
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MAX_VALUE + 1)), false, file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", INT_MAX_VALUE + 6)), false)
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, not(equal("ID", INT_MAX_VALUE + 6)), false, file());
assertThat(shouldRead).as("Should read: id above upper bound").isTrue();
}
@Test
public void testCaseSensitiveIntegerNotEqRewritten() {
- assertThatThrownBy(
- () -> new InclusiveMetricsEvaluator(SCHEMA, not(equal("ID", 5)), true).eval(FILE))
+ assertThatThrownBy(() -> shouldRead(SCHEMA, not(equal("ID", 5)), true, file()))
.isInstanceOf(ValidationException.class)
.hasMessageContaining("Cannot find field 'ID'");
}
@Test
public void testStringStartsWith() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "a"), true).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, startsWith("required", "a"), true, file());
assertThat(shouldRead).as("Should read: no stats").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "a"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "a"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "aa"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "aa"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "aaa"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "aaa"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "1s"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "1s"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "1str1x"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "1str1x"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "ff"), true).eval(FILE_4);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "ff"), true, file4());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "aB"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "aB"), true, file2());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "dWX"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "dWX"), true, file2());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "5"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "5"), true, file3());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", "3str3x"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", "3str3x"), true, file3());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("some_empty", "房东整租霍"), true).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, startsWith("some_empty", "房东整租霍"), true, file());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("all_nulls", ""), true).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, startsWith("all_nulls", ""), true, file());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
String aboveMax = UnicodeUtil.truncateStringMax(Literal.of("イロハニホヘト"), 4).value().toString();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, startsWith("required", aboveMax), true).eval(FILE_4);
+ shouldRead = shouldRead(SCHEMA, startsWith("required", aboveMax), true, file4());
assertThat(shouldRead).as("Should not read: range doesn't match").isFalse();
}
@Test
public void testStringNotStartsWith() {
- boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "a"), true).eval(FILE);
+ boolean shouldRead = shouldRead(SCHEMA, notStartsWith("required", "a"), true, file());
assertThat(shouldRead).as("Should read: no stats").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "a"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "a"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "aa"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "aa"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "aaa"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "aaa"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "1s"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "1s"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "1str1x"), true)
- .eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "1str1x"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "ff"), true).eval(FILE_4);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "ff"), true, file4());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "aB"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "aB"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "dWX"), true).eval(FILE_2);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "dWX"), true, file2());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "5"), true).eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "5"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "3str3x"), true)
- .eval(FILE_3);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "3str3x"), true, file3());
assertThat(shouldRead).as("Should read: range matches").isTrue();
String aboveMax = UnicodeUtil.truncateStringMax(Literal.of("イロハニホヘト"), 4).value().toString();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", aboveMax), true)
- .eval(FILE_4);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", aboveMax), true, file4());
assertThat(shouldRead).as("Should read: range matches").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "abc"), true).eval(FILE_5);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "abc"), true, file5());
assertThat(shouldRead).as("Should not read: all strings start with prefix").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notStartsWith("required", "abcd"), true).eval(FILE_5);
+ shouldRead = shouldRead(SCHEMA, notStartsWith("required", "abcd"), true, file5());
assertThat(shouldRead).as("Should not read: lower shorter than prefix, cannot match").isTrue();
}
@Test
public void testIntegerIn() {
boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MIN_VALUE - 25, INT_MIN_VALUE - 24))
- .eval(FILE);
+ shouldRead(SCHEMA, in("id", INT_MIN_VALUE - 25, INT_MIN_VALUE - 24), file());
assertThat(shouldRead).as("Should not read: id below lower bound (5 < 30, 6 < 30)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MIN_VALUE - 2, INT_MIN_VALUE - 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MIN_VALUE - 2, INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should not read: id below lower bound (28 < 30, 29 < 30)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MIN_VALUE - 1, INT_MIN_VALUE))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MIN_VALUE - 1, INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to lower bound (30 == 30)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MAX_VALUE - 4, INT_MAX_VALUE - 3))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MAX_VALUE - 4, INT_MAX_VALUE - 3), file());
assertThat(shouldRead)
.as("Should read: id between lower and upper bounds (30 < 75 < 79, 30 < 76 < 79)")
.isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MAX_VALUE, INT_MAX_VALUE + 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MAX_VALUE, INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should read: id equal to upper bound (79 == 79)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MAX_VALUE + 1, INT_MAX_VALUE + 2))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MAX_VALUE + 1, INT_MAX_VALUE + 2), file());
assertThat(shouldRead).as("Should not read: id above upper bound (80 > 79, 81 > 79)").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, in("id", INT_MAX_VALUE + 6, INT_MAX_VALUE + 7))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", INT_MAX_VALUE + 6, INT_MAX_VALUE + 7), file());
assertThat(shouldRead).as("Should not read: id above upper bound (85 > 79, 86 > 79)").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, in("all_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("all_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should skip: in on all nulls column").isFalse();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, in("some_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("some_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: in on some nulls column").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, in("no_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("no_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: in on no nulls column").isTrue();
// should read as the number of elements in the in expression is too big
List<Integer> ids = Lists.newArrayListWithExpectedSize(400);
for (int id = -400; id <= 0; id++) {
ids.add(id);
}
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, in("id", ids)).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, in("id", ids), file());
assertThat(shouldRead).as("Should read: large in expression").isTrue();
}
@Test
public void testIntegerNotIn() {
boolean shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MIN_VALUE - 25, INT_MIN_VALUE - 24))
- .eval(FILE);
+ shouldRead(SCHEMA, notIn("id", INT_MIN_VALUE - 25, INT_MIN_VALUE - 24), file());
assertThat(shouldRead).as("Should read: id below lower bound (5 < 30, 6 < 30)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MIN_VALUE - 2, INT_MIN_VALUE - 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MIN_VALUE - 2, INT_MIN_VALUE - 1), file());
assertThat(shouldRead).as("Should read: id below lower bound (28 < 30, 29 < 30)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MIN_VALUE - 1, INT_MIN_VALUE))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MIN_VALUE - 1, INT_MIN_VALUE), file());
assertThat(shouldRead).as("Should read: id equal to lower bound (30 == 30)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MAX_VALUE - 4, INT_MAX_VALUE - 3))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MAX_VALUE - 4, INT_MAX_VALUE - 3), file());
assertThat(shouldRead)
.as("Should read: id between lower and upper bounds (30 < 75 < 79, 30 < 76 < 79)")
.isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MAX_VALUE, INT_MAX_VALUE + 1))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MAX_VALUE, INT_MAX_VALUE + 1), file());
assertThat(shouldRead).as("Should read: id equal to upper bound (79 == 79)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MAX_VALUE + 1, INT_MAX_VALUE + 2))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MAX_VALUE + 1, INT_MAX_VALUE + 2), file());
assertThat(shouldRead).as("Should read: id above upper bound (80 > 79, 81 > 79)").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("id", INT_MAX_VALUE + 6, INT_MAX_VALUE + 7))
- .eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("id", INT_MAX_VALUE + 6, INT_MAX_VALUE + 7), file());
assertThat(shouldRead).as("Should read: id above upper bound (85 > 79, 86 > 79)").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, notIn("all_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("all_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: notIn on all nulls column").isTrue();
- shouldRead =
- new InclusiveMetricsEvaluator(SCHEMA, notIn("some_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("some_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: notIn on some nulls column").isTrue();
- shouldRead = new InclusiveMetricsEvaluator(SCHEMA, notIn("no_nulls", "abc", "def")).eval(FILE);
+ shouldRead = shouldRead(SCHEMA, notIn("no_nulls", "abc", "def"), file());
assertThat(shouldRead).as("Should read: notIn on no nulls column").isTrue();
}
@Test
public void testIsNullInNestedStruct() {
// read required_address and its nested fields
- boolean shouldRead =
- new InclusiveMetricsEvaluator(NESTED_SCHEMA, isNull("required_address")).eval(FILE_6);
+ boolean shouldRead = shouldRead(NESTED_SCHEMA, isNull("required_address"), file6());
assertThat(shouldRead).as("Should not read: required_address is required").isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(NESTED_SCHEMA, isNull("required_address.required_street1"))
- .eval(FILE_6);
+ shouldRead = shouldRead(NESTED_SCHEMA, isNull("required_address.required_street1"), file6());
assertThat(shouldRead)
.as("Should not read: required_address.required_street1 is required")
.isFalse();
- shouldRead =
- new InclusiveMetricsEvaluator(NESTED_SCHEMA, isNull("required_address.optional_street1"))
- .eval(FILE_6);
+ shouldRead = shouldRead(NESTED_SCHEMA, isNull("required_address.optional_street1"), file6());
assertThat(shouldRead)
.as("Should read: required_address.optional_street1 is optional")
.isTrue();
// read optional_address and its nested fields
- shouldRead =
- new InclusiveMetricsEvaluator(NESTED_SCHEMA, isNull("optional_address")).eval(FILE_6);
+ shouldRead = shouldRead(NESTED_SCHEMA, isNull("optional_address"), file6());
Review Comment:
Is this overlapping with https://github.com/apache/iceberg/pull/17412?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #17430:
URL: https://github.com/apache/iceberg/pull/17430#discussion_r3684380092
##########
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluator.java:
##########
@@ -210,150 +212,303 @@ public class TestInclusiveMetricsEvaluator {
Row.of(),
10,
// any value counts, including nulls
- ImmutableMap.of(100, 5L, 101, 5L, 102, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(102, 5L, 103, 5L, 104, 5L, 105, 5L),
// null value counts
- ImmutableMap.of(100, 0L, 101, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(103, 5L, 104, 5L, 105, 5L),
// nan value counts
null,
// lower bounds
null,
// upper bounds
null);
+ private static final DataFile MISSING_STATS = new TestDataFile("file.parquet", Row.of(), 50);
+
+ private static final DataFile EMPTY_FILE = new TestDataFile("file.parquet", Row.of(), 0);
+
+ private static final DataFile RANGE_OF_VALUES =
+ new TestDataFile(
+ "range_of_values.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(3, 10L),
+ ImmutableMap.of(3, 0L),
+ null,
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "aaa")),
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "zzz")));
+
+ private static final DataFile SINGLE_VALUE_FILE =
+ new TestDataFile(
+ "single_value.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(3, 10L),
+ ImmutableMap.of(3, 0L),
+ null,
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "abc")),
+ ImmutableMap.of(3, toByteBuffer(StringType.get(), "abc")));
+
+ // some_empty is optional because a required column cannot contain nulls
+ private static final DataFile SINGLE_VALUE_WITH_NULLS =
+ new TestDataFile(
+ "single_value_nulls.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(14, 10L),
+ ImmutableMap.of(14, 2L),
+ null,
+ ImmutableMap.of(14, toByteBuffer(StringType.get(), "abc")),
+ ImmutableMap.of(14, toByteBuffer(StringType.get(), "abc")));
+
+ private static final DataFile SINGLE_VALUE_WITH_NAN =
+ new TestDataFile(
+ "single_value_nan.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(9, 10L),
+ ImmutableMap.of(9, 0L),
+ ImmutableMap.of(9, 2L),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), 5.0F)),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), 5.0F)));
+
+ private static final DataFile SINGLE_VALUE_NAN_BOUNDS =
+ new TestDataFile(
+ "single_value_nan_bounds.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(9, 10L),
+ ImmutableMap.of(9, 0L),
+ ImmutableMap.of(9, 0L),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), Float.NaN)),
+ ImmutableMap.of(9, toByteBuffer(Types.FloatType.get(), Float.NaN)));
+
+ private static final Map<Integer, ByteBuffer> FLOAT_BOUND =
+ ImmutableMap.of(1, toByteBuffer(Types.FloatType.get(), 1.0f));
+
+ private static final DataFile SINGLE_FLOAT_VALUE_FILE =
+ new TestDataFile(
+ "single_value_file.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(1, 10L),
+ ImmutableMap.of(1, 0L),
+ ImmutableMap.of(1, 0L),
+ FLOAT_BOUND,
+ FLOAT_BOUND);
+
+ private static final DataFile SINGLE_FLOAT_VALUE_FILE_WITH_NAN =
+ new TestDataFile(
+ "single_value_file.avro",
+ Row.of(),
+ 10,
+ ImmutableMap.of(1, 10L),
+ ImmutableMap.of(1, 0L),
+ ImmutableMap.of(1, 1L), // contains a NaN value
+ FLOAT_BOUND,
+ FLOAT_BOUND);
+
+ protected boolean shouldRead(Schema schema, Expression expr, boolean caseSensitive, F testFile) {
+ return new InclusiveMetricsEvaluator(schema, expr, caseSensitive).eval((DataFile) testFile);
+ }
+
+ protected boolean shouldRead(Schema schema, Expression expr, F testFile) {
+ return shouldRead(schema, expr, true, testFile);
+ }
+
+ protected F file() {
+ return asFile(FILE);
+ }
+
+ protected F file2() {
+ return asFile(FILE_2);
+ }
+
+ protected F file3() {
+ return asFile(FILE_3);
+ }
+
+ protected F file4() {
+ return asFile(FILE_4);
+ }
+
+ protected F file5() {
+ return asFile(FILE_5);
+ }
+
+ protected F file6() {
+ return asFile(FILE_6);
+ }
+
+ protected F missingStats() {
+ return asFile(MISSING_STATS);
+ }
+
+ protected F emptyFile() {
+ return asFile(EMPTY_FILE);
+ }
+
+ protected F rangeOfValues() {
+ return asFile(RANGE_OF_VALUES);
+ }
+
+ protected F singleValueFile() {
+ return asFile(SINGLE_VALUE_FILE);
+ }
+
+ protected F singleValueWithNulls() {
+ return asFile(SINGLE_VALUE_WITH_NULLS);
+ }
+
+ protected F singleValueWithNaN() {
+ return asFile(SINGLE_VALUE_WITH_NAN);
+ }
+
+ protected F singleValueNaNBounds() {
+ return asFile(SINGLE_VALUE_NAN_BOUNDS);
+ }
+
+ protected F singleFloatValueFile() {
+ return asFile(SINGLE_FLOAT_VALUE_FILE);
+ }
+
+ protected F singleFloatValueFileWithNaN() {
+ return asFile(SINGLE_FLOAT_VALUE_FILE_WITH_NAN);
+ }
+
+ @SuppressWarnings("unchecked")
Review Comment:
Nit: the 15 protected factory methods each delegate to the private asFile(DataFile), which performs an unchecked (F) dataFile cast. A subclass that extends the base with a non-DataFile type F (e.g. a ContentFile variant) and fails to override even one factory method will receive a runtime ClassCastException with no compile-time signal; the type parameter F has no bound, so the compiler cannot catch it. The @SuppressWarnings("unchecked") on asFile() is correct per convention, but the contract ("subclasses using a non-DataFile F must override every factory method and both shouldRead() overloads") is nowhere documented. A Javadoc block on the class and/or on asFile() noting this is the recommended fix.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
Re: [PR] API, Core: Make TestInclusiveMetricsEvaluator/TestInclusiveMetricsEvaluatorWithExtract/TestInclusiveMetricsEvaluatorWithTransforms extendable [iceberg]
Posted by "nastra (via GitHub)" <gi...@apache.org>.
nastra commented on code in PR #17430:
URL: https://github.com/apache/iceberg/pull/17430#discussion_r3694978858
##########
api/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluator.java:
##########
@@ -210,150 +212,303 @@ public class TestInclusiveMetricsEvaluator {
Row.of(),
10,
// any value counts, including nulls
- ImmutableMap.of(100, 5L, 101, 5L, 102, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(102, 5L, 103, 5L, 104, 5L, 105, 5L),
// null value counts
- ImmutableMap.of(100, 0L, 101, 5L, 103, 5L, 104, 5L, 105, 5L),
+ ImmutableMap.of(103, 5L, 104, 5L, 105, 5L),
Review Comment:
yep, I've rebased the PR now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org