stevenzwu commented on code in PR #6402: URL: https://github.com/apache/iceberg/pull/6402#discussion_r1045171171
########## flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/source/TestFlinkTableSource.java: ########## @@ -96,6 +97,7 @@ public void before() { @After public void clean() { sql("DROP TABLE IF EXISTS %s.%s", DATABASE_NAME, TABLE_NAME); + sql("DROP TABLE IF EXISTS %s.%s", DATABASE_NAME, TABLE_NAME_NAN); Review Comment: might be better just do it with try-finally in the `testFilterNaN` method, since this is not applicable to other tests. save one sql run for other tests. ########## flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/source/TestFlinkTableSource.java: ########## @@ -603,7 +605,103 @@ public void testFilterPushDown2Literal() { } @Test - public void testSqlParseNaN() { - // todo add some test case to test NaN + public void testFilterNaN() { + sql( + "CREATE TABLE %s (id INT, data VARCHAR,d DOUBLE, f FLOAT) WITH ('write.format.default'='%s')", Review Comment: probably not necessary to have the 4th `f FLOAT` column. ########## flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/source/TestFlinkTableSource.java: ########## @@ -603,7 +605,103 @@ public void testFilterPushDown2Literal() { } @Test - public void testSqlParseNaN() { - // todo add some test case to test NaN + public void testFilterNaN() { + sql( + "CREATE TABLE %s (id INT, data VARCHAR,d DOUBLE, f FLOAT) WITH ('write.format.default'='%s')", + TABLE_NAME_NAN, format.name()); + sql( + "INSERT INTO %s VALUES (1,'iceberg',10, 1.1),(2,'b',20,2.2),(3,CAST(NULL AS VARCHAR),30,3.3),(4,'d',CAST('NaN' AS DOUBLE),4.4)", + TABLE_NAME_NAN); + + String sqlNaNDoubleEqual = + String.format("SELECT * FROM %s WHERE d = CAST('NaN' AS DOUBLE) ", TABLE_NAME_NAN); + List<Row> resultDoubleEqual = sql(sqlNaNDoubleEqual); + Assert.assertEquals("Should have 0 records", 0, resultDoubleEqual.size()); Review Comment: In theory, this should have been 1. When I investigated this myself a few weeks ago, I ran into the problem. `NaN` where clause didn't work correctly. not sure where is the problem. We can add some comments to explain the unexpected behavior and keep this test as it is for now. We can follow up on this with a separate PR, which requires deeper dig. ########## flink/v1.16/flink/src/test/java/org/apache/iceberg/flink/source/TestFlinkTableSource.java: ########## @@ -603,7 +605,103 @@ public void testFilterPushDown2Literal() { } @Test - public void testSqlParseNaN() { - // todo add some test case to test NaN + public void testFilterNaN() { + sql( + "CREATE TABLE %s (id INT, data VARCHAR,d DOUBLE, f FLOAT) WITH ('write.format.default'='%s')", + TABLE_NAME_NAN, format.name()); + sql( + "INSERT INTO %s VALUES (1,'iceberg',10, 1.1),(2,'b',20,2.2),(3,CAST(NULL AS VARCHAR),30,3.3),(4,'d',CAST('NaN' AS DOUBLE),4.4)", + TABLE_NAME_NAN); + + String sqlNaNDoubleEqual = + String.format("SELECT * FROM %s WHERE d = CAST('NaN' AS DOUBLE) ", TABLE_NAME_NAN); + List<Row> resultDoubleEqual = sql(sqlNaNDoubleEqual); + Assert.assertEquals("Should have 0 records", 0, resultDoubleEqual.size()); + + String sqlNaNDoubleNotEqual = + String.format("SELECT * FROM %s WHERE d <> CAST('NaN' AS DOUBLE) ", TABLE_NAME_NAN); + List<Row> resultDoubleNotEqual = sql(sqlNaNDoubleNotEqual); + List<Row> expectedDouble = + Lists.newArrayList( + Row.of(1, "iceberg", 10.0d, 1.1f), + Row.of(2, "b", 20.0d, 2.2f), + Row.of(3, null, 30.0d, 3.3f), + Row.of(4, "d", Double.NaN, 4.4f)); + Assert.assertEquals("Should have 4 records", 4, resultDoubleNotEqual.size()); Review Comment: same problem for `NaN` where clause. in theory, this should have been 3. -- 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-unsubscr...@iceberg.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org For additional commands, e-mail: issues-h...@iceberg.apache.org