uros-b commented on code in PR #17903:
URL: https://github.com/apache/iceberg/pull/17903#discussion_r3903028474
##########
flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/source/TestFlinkTableSource.java:
##########
@@ -556,6 +556,26 @@ public void testFilterPushDown2Literal() {
@TestTemplate
public void testSqlParseNaN() {
Review Comment:
The test dataset contains no NaN rows (d = 10/20/30 only), so is_nan ->
isEmpty() and not_nan -> hasSize(3) are both trivially satisfied regardless of
predicate correctness; adding a row with d = Double.NaN would let both
predicates demonstrate actual selective filtering and would materially
strengthen the coverage claim.
##########
flink/v1.20/flink/src/test/java/org/apache/iceberg/flink/source/TestFlinkTableSource.java:
##########
@@ -556,6 +556,26 @@ public void testFilterPushDown2Literal() {
@TestTemplate
public void testSqlParseNaN() {
Review Comment:
This goes for all 4 version dirs
##########
flink/v2.1/flink/src/test/java/org/apache/iceberg/flink/source/TestFlinkTableSource.java:
##########
@@ -556,6 +556,26 @@ public void testFilterPushDown2Literal() {
@TestTemplate
public void testSqlParseNaN() {
- // todo add some test case to test NaN
+ String sqlEqual =
+ String.format("SELECT * FROM %s WHERE d = CAST('NaN' AS DOUBLE) ",
TABLE_NAME);
+ String expectedFilterEqual = "is_nan(ref(name=\"d\"))";
+
+ List<Row> resultEqual = sql(sqlEqual);
+ assertThat(resultEqual).isEmpty();
+ assertThat(lastScanEvent.filter())
+ .as("Should contain the pushed down NaN filter")
+ .asString()
+ .isEqualTo(expectedFilterEqual);
+
+ String sqlNotEqual =
+ String.format("SELECT * FROM %s WHERE d <> CAST('NaN' AS DOUBLE) ",
TABLE_NAME);
+ String expectedFilterNotEqual = "not_nan(ref(name=\"d\"))";
+
+ List<Row> resultNotEqual = sql(sqlNotEqual);
+ assertThat(resultNotEqual).hasSize(3);
Review Comment:
Regarding the resultNotEqual assertion, only hasSize(3) is checked here, not
row identity. However, every other multi-row pushdown test in the file calls
assertSameElements(expectedList, result) (e.g. testFilterPushDownNoEqual);
matching that pattern would be file-consistent and strictly stronger.
--
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]
