Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/18399 )
Change subject: IMPALA-10850: Interpret timestamp predicates in local timezone in IcebergScanNode ...................................................................... Patch Set 6: (4 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/18399/6/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java File fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java: http://gerrit.cloudera.org:8080/#/c/18399/6/fe/src/main/java/org/apache/impala/planner/IcebergScanNode.java@31 PS6, Line 31: import org.apache.iceberg.expressions.UnboundPredicate; : import org.apache.hadoop.fs.Path; > nit: order of imports looks wrong Done http://gerrit.cloudera.org:8080/#/c/18399/6/testdata/data/README File testdata/data/README: http://gerrit.cloudera.org:8080/#/c/18399/6/testdata/data/README@722 PS6, Line 722: insert into iceberg_timestamp_part values (1, '2021-10-31 02:15:00'), (2, '2021-01-10 12:00:00'), (3, '2022-04-11 00:04:00'), (4, '2022-04-11 12:04:55'); > It would be nice to add 1-2 <1970-01-01 timestamps to give coverage for ne Done http://gerrit.cloudera.org:8080/#/c/18399/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test: http://gerrit.cloudera.org:8080/#/c/18399/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test@551 PS6, Line 551: WHERE ts = '2021-10-31 00:15:00'; > Can you also add some tests with < or > predicates? Done http://gerrit.cloudera.org:8080/#/c/18399/6/testdata/workloads/functional-query/queries/QueryTest/iceberg-query.test@615 PS6, Line 615: aggregation(SUM, NumRowGroups): 5 > Hmm, I wonder why we don't drop some of these during stat filtering. Maybe In my case the stats are: - NumFileMetadataRead: 0 (0) - NumRowGroups: 5 (5) - NumStatsFilteredRowGroups: 3 (3) NumFileMetadataRead: 0 is strange since we definitely read the metadata to filter out row groups. Iceberg tables are treated as non-partitioned in most cases. We let Iceberg filter out partitions during planning. When predicate pushdown doesn't work (like in this test) it'd make sense to evaluate the stats for identity-partitioned tables (using the template tuple), but these tables are partitioned via the HOUR partition transform. -- To view, visit http://gerrit.cloudera.org:8080/18399 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I181be5d2fa004f69b457f69ff82dc2f9877f46fa Gerrit-Change-Number: 18399 Gerrit-PatchSet: 6 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tamas Mate <tma...@apache.org> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Tue, 19 Apr 2022 16:59:58 +0000 Gerrit-HasComments: Yes