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

Reply via email to