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

Reply via email to