huan233usc commented on code in PR #16570:
URL: https://github.com/apache/iceberg/pull/16570#discussion_r3336841513
##########
spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/source/TestFilteredScan.java:
##########
@@ -269,6 +271,53 @@ public void testUnpartitionedTimestampFilter() {
"ts < cast('2017-12-22 00:00:00+00:00' as timestamp)"));
}
+ @TestTemplate
Review Comment:
Do you want to test hashcode()?
##########
spark/v4.1/spark/src/main/java/org/apache/iceberg/spark/source/SparkRuntimeFilterableScan.java:
##########
@@ -195,6 +195,6 @@ private Expression convertRuntimePredicates(Predicate[]
predicates) {
}
protected String runtimeFiltersDesc() {
- return Spark3Util.describe(runtimeFilters);
+ return createOrderedExprString(runtimeFilters.stream());
Review Comment:
Will this affect the behavior of `EXPLAIN` statement?
The output format changes from (a = 1 AND b = 2), c = 3 (tree-preserving) to
a = 1, b = 2, c = 3 (flattened + sorted).
Can we separate the method for powering `EXPLAIN` and hashcode etc?
##########
spark/v4.1/spark/src/test/java/org/apache/iceberg/spark/sql/TestFilterPushDown.java:
##########
@@ -674,9 +683,14 @@ private void checkFilters(
assertThat(planAsString).as("Should be no post scan
filter").doesNotContain("Filter (");
}
- assertThat(planAsString)
- .as("Pushed filters must match")
- .contains(", filters=" + icebergFilters + ",");
+ int startIndex = planAsString.indexOf("filters=");
+ int endIndex = planAsString.indexOf("runtimeFilters");
+ String filterStringFromPlan = planAsString.substring(startIndex, endIndex);
+ Arrays.stream(icebergFilters)
Review Comment:
Will this change make `checkFilters` lost the ability to test the case I
expect no filter to push down? e.g. I could pass empty for all the tests and it
passes always.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
