Qifan Chen has posted comments on this change. ( http://gerrit.cloudera.org:8080/17765 )
Change subject: IMPALA-10840: Add support for "FOR SYSTEM_TIME AS OF" and "FOR SYSTEM_VERSION AS OF" for Iceberg tables ...................................................................... Patch Set 4: (5 comments) Looks great! We should also state that translating the local time to the time zone used for all the rows in the table so that the result is correct WRT to the SQL standard. http://gerrit.cloudera.org:8080/#/c/17765/4//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17765/4//COMMIT_MSG@14 PS4, Line 14: May need to describe the semantics of FOR SYSTEM_TIME AS OF and FOR SYSTEM_VERSION AS OF" here. http://gerrit.cloudera.org:8080/#/c/17765/4//COMMIT_MSG@18 PS4, Line 18: local timezone. nit. Translating the local time to the time zone used for all the rows in the table is critical. http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java File fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java: http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/main/java/org/apache/impala/analysis/TimeTravelSpec.java@39 PS4, Line 39: TIME nit. May use TIME_AS_OF. Other possibilities exist, such as TIME_FROM or TIME_BETWEEN http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4871 PS4, Line 4871: iceT); nit. May add a test for a time in future, say 3 days from now(). http://gerrit.cloudera.org:8080/#/c/17765/4/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@4880 PS4, Line 4880: nit. May add a negative test case on system version of -10. -- To view, visit http://gerrit.cloudera.org:8080/17765 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib523c5e47b8d9c377bea39a82fe20249177cf824 Gerrit-Change-Number: 17765 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 26 Aug 2021 18:45:39 +0000 Gerrit-HasComments: Yes
