Amogh Margoor 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 5: Code-Review+1 (3 comments) nice addition. LGTM! http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py File tests/query_test/test_iceberg.py: http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@116 PS5, Line 116: def test_time_travel(self, vector, unique_database): > Snapshots should be available until they are expired: One more case that can be added to JIRA above is time travel's behaviour with schema evolution. I guess with old snapshot selected old schema is picked up too. http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@171 PS5, Line 171: # Future queries return the current snapshot. > No, you can only query concrete snapshots. Also, the snapshot IDs are not i ahh I see.. didn't know snapshot ids are not monotonically increasing. http://gerrit.cloudera.org:8080/#/c/17765/5/tests/query_test/test_iceberg.py@205 PS5, Line 205: self.execute_query("SELECT * FROM {0} FOR SYSTEM_TIME AS OF {1}".format( > Yes, but it's not easy to add a test for it currently. It will be covered b makes sense. -- 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: 5 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Amogh Margoor <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Qifan Chen <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Fri, 27 Aug 2021 16:03:08 +0000 Gerrit-HasComments: Yes
