You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/12/12 21:37:07 UTC
[GitHub] [iceberg] stevenzwu commented on a diff in pull request #6402: Flink: Add UT for NaN
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-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