You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Csaba Ringhofer (Code Review)" <ge...@cloudera.org> on 2022/03/01 13:05:25 UTC

[Impala-ASF-CR] IMPALA-11053: Impala should be able to read migrated partitioned Iceberg tables

Csaba Ringhofer has posted comments on this change. ( http://gerrit.cloudera.org:8080/18240 )

Change subject: IMPALA-11053: Impala should be able to read migrated partitioned Iceberg tables
......................................................................


Patch Set 4:

(4 comments)

Looks superb, few comments about tests.

http://gerrit.cloudera.org:8080/#/c/18240/4/common/fbs/IcebergObjects.fbs
File common/fbs/IcebergObjects.fbs:

http://gerrit.cloudera.org:8080/#/c/18240/4/common/fbs/IcebergObjects.fbs@23
PS4, Line 23:   AVRO
AVRO was added here as a possibility, but there is no test for it yet (if I haven't missed something). It is also not mentioned in the commit mesage.


http://gerrit.cloudera.org:8080/#/c/18240/4/testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_alltypes_part/metadata/v1.metadata.json
File testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_alltypes_part/metadata/v1.metadata.json:

http://gerrit.cloudera.org:8080/#/c/18240/4/testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_alltypes_part/metadata/v1.metadata.json@1
PS4, Line 1: {
Can you add some minimal info about these files to https://github.com/apache/impala/blob/master/testdata/data/README ?


http://gerrit.cloudera.org:8080/#/c/18240/4/testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_alltypes_part/metadata/v1.metadata.json@148
PS4, Line 148:     "engine.hive.enabled" : "true",
             :     "MIGRATED_TO_ICEBERG" : "true",
Does the Hive we depend on support upgrading to Iceberg?
It would be great to add a test for the interop scenario where  the Iceberg metadata files are created by Hive during the test or dataload. This would allow us to catch if something changes on the Hive side.


http://gerrit.cloudera.org:8080/#/c/18240/4/testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_legacy_partition_schema_evolution_orc/metadata/v1.metadata.json
File testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_legacy_partition_schema_evolution_orc/metadata/v1.metadata.json:

http://gerrit.cloudera.org:8080/#/c/18240/4/testdata/data/iceberg_test/hadoop_catalog/ice/iceberg_legacy_partition_schema_evolution_orc/metadata/v1.metadata.json@77
PS4, Line 77:     "write.format.default" : "parquet",
This name of the table contains "orc", but metadata files contain "parquet"



-- 
To view, visit http://gerrit.cloudera.org:8080/18240
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac11a02de709d43532056f71359c49d20c1be2b8
Gerrit-Change-Number: 18240
Gerrit-PatchSet: 4
Gerrit-Owner: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gf...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Zoltan Borok-Nagy <bo...@cloudera.com>
Gerrit-Comment-Date: Tue, 01 Mar 2022 13:05:25 +0000
Gerrit-HasComments: Yes