wangsheng has posted comments on this change. ( http://gerrit.cloudera.org:8080/16599 )
Change subject: IMPALA-10153: Implement SHOW TABLE HISTORY for Iceberg tables ...................................................................... Patch Set 1: (7 comments) Thanks for the new feature! http://gerrit.cloudera.org:8080/#/c/16599/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16599/1//COMMIT_MSG@7 PS1, Line 7: IMPALA-10153 I think maybe we should create a new sub-task under IMPALA-10153 for this path, only for 'SHOW TABLE HISTORY' SQL supported, instead of use this JIRA directly. Since there maybe other patch related to time travel would be submitted in the future. http://gerrit.cloudera.org:8080/#/c/16599/1//COMMIT_MSG@14 PS1, Line 14: +----------------------------+---------------------+---------------------+---------------------+ : | creation_time | snapshot_id | parent_id | is_current_ancestor | : +----------------------------+---------------------+---------------------+---------------------+ : | 2020-10-13 14:01:07.234000 | 4400379706200951771 | NULL | TRUE | : | 2020-10-13 14:01:19.307000 | 4221472712544505868 | 4400379706200951771 | TRUE | : +----------------------------+---------------------+---------------------+---------------------+ Maybe we can add more info here, such as operation/manifest_list/summary. Spark supported sql like this 'SELECT * FROM prod.db.table.snapshots' to get snapshot, you can refer to this doc: http://iceberg.apache.org/spark/ http://gerrit.cloudera.org:8080/#/c/16599/1/be/src/service/client-request-state.cc File be/src/service/client-request-state.cc: http://gerrit.cloudera.org:8080/#/c/16599/1/be/src/service/client-request-state.cc@426 PS1, Line 426: case TCatalogOpType::SHOW_TABLE_HISTORY Maybe we should add some comments here, since this syntax is only supported for Iceberg tables now. http://gerrit.cloudera.org:8080/#/c/16599/1/common/thrift/Frontend.thrift File common/thrift/Frontend.thrift: http://gerrit.cloudera.org:8080/#/c/16599/1/common/thrift/Frontend.thrift@287 PS1, Line 287: struct TGetTableHistoryResult { : 1: required list<TGetTableHistoryResultItem> result : } : : struct TGetTableHistoryResultItem { Please also add some comments for these new thrift structs. http://gerrit.cloudera.org:8080/#/c/16599/1/fe/src/main/java/org/apache/impala/service/Frontend.java File fe/src/main/java/org/apache/impala/service/Frontend.java: http://gerrit.cloudera.org:8080/#/c/16599/1/fe/src/main/java/org/apache/impala/service/Frontend.java@109 PS1, Line 109: import org.apache.impala.catalog.IcebergTable; Unused import http://gerrit.cloudera.org:8080/#/c/16599/1/fe/src/main/java/org/apache/impala/service/Frontend.java@1066 PS1, Line 1066: public TGetTableHistoryResult getTableHistory(String dbName, String tableName) Please also add some comments for this method. http://gerrit.cloudera.org:8080/#/c/16599/1/testdata/datasets/functional/functional_schema_template.sql File testdata/datasets/functional/functional_schema_template.sql: http://gerrit.cloudera.org:8080/#/c/16599/1/testdata/datasets/functional/functional_schema_template.sql@2983 PS1, Line 2983: /test-warehouse/iceberg_multi_snapshots Maybe we should use '/test-warehouse/iceberg_test/iceberg_multi_snapshots' to keep consist of above Iceberg tables. -- To view, visit http://gerrit.cloudera.org:8080/16599 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I56a4b92c27e8e4a79109696cbae62735a00750e5 Gerrit-Change-Number: 16599 Gerrit-PatchSet: 1 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Thu, 15 Oct 2020 06:59:32 +0000 Gerrit-HasComments: Yes
