You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "zhongyujiang (via GitHub)" <gi...@apache.org> on 2023/04/14 13:13:41 UTC

[GitHub] [iceberg] zhongyujiang opened a new pull request, #7346: API: Binder should not simplify isNull and notNull when their target …

zhongyujiang opened a new pull request, #7346:
URL: https://github.com/apache/iceberg/pull/7346

   …fields are required but nested in optional parents.
   
   Currently, `Binder` will simpify `isNull` and `notNull` to `alwaysFalse` and `alwaysTrue` respectively, when the expressions are to be bound to required fields. However a required field can still be null in the row when it is nested in an optional parent field. Therefore, this causes data to be incorrectly skipped when querying for rows where their nested field(required sub field nested in optional parent) is null.
   
   This changes the `Binder` to only simplify nullability predicates when their target field and all its parents are all required, to fix this issue.


-- 
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


[GitHub] [iceberg] aokolnychyi commented on pull request #7346: API: Binder should not simplify isNull and notNull when their target …

Posted by "aokolnychyi (via GitHub)" <gi...@apache.org>.
aokolnychyi commented on PR #7346:
URL: https://github.com/apache/iceberg/pull/7346#issuecomment-1512507108

   Seems like a good catch. I'll try to review the actual fix later this week unless someone gets there earlier.
   
   cc @nastra @rdblue @szehon-ho @flyrain @amogh-jahagirdar @jackye1995 


-- 
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


[GitHub] [iceberg] zhongyujiang commented on pull request #7346: API: Binder should not simplify isNull and notNull when their target …

Posted by "zhongyujiang (via GitHub)" <gi...@apache.org>.
zhongyujiang commented on PR #7346:
URL: https://github.com/apache/iceberg/pull/7346#issuecomment-1534634109

   Hey @aokolnychyi , do you have time to review this?


-- 
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


[GitHub] [iceberg] zhongyujiang commented on a diff in pull request #7346: API: Binder should not simplify isNull and notNull when their target …

Posted by "zhongyujiang (via GitHub)" <gi...@apache.org>.
zhongyujiang commented on code in PR #7346:
URL: https://github.com/apache/iceberg/pull/7346#discussion_r1166823764


##########
spark/v3.3/spark/src/test/java/org/apache/iceberg/spark/sql/TestFilterPushDown.java:
##########
@@ -516,6 +516,25 @@ public void testFilterPushdownWithSpecialFloatingPointPartitionValues() {
         ImmutableList.of(row(4, Double.NEGATIVE_INFINITY)));
   }
 
+  @Test
+  public void testNullabilityPredicatesOnNestedFieldsPushDown() {
+    sql(
+        "CREATE TABLE %s (id INT, point struct<x: BIGINT NOT NULL, y: BIGINT NOT NULL>)"
+            + "USING iceberg ",
+        tableName);
+    sql("INSERT INTO %s VALUES(1, struct(0, 0)), (2, null)", tableName);
+
+    checkOnlyIcebergFilters(
+        "point.x IS NULL" /* query predicate */,
+        "point.x IS NULL" /* Iceberg scan filters */,

Review Comment:
   This query returns no rows without this PR.



-- 
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