Tamas Mate has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/19353 )

Change subject: IMPALA-11708: Add support for mixed Iceberg tables with AVRO 
file format
......................................................................


Patch Set 1:

(1 comment)

Nice improvement! I only had a code organisation question.

http://gerrit.cloudera.org:8080/#/c/19353/1/be/src/exec/hdfs-scan-node-base.cc
File be/src/exec/hdfs-scan-node-base.cc:

http://gerrit.cloudera.org:8080/#/c/19353/1/be/src/exec/hdfs-scan-node-base.cc@306
PS1, Line 306:         if (file_metadata) {
             :           switch 
(file_metadata->iceberg_metadata()->file_format()) {
             :             case 
FbIcebergDataFileFormat::FbIcebergDataFileFormat_PARQUET:
             :               file_desc->file_format = THdfsFileFormat::PARQUET;
             :               break;
             :             case 
FbIcebergDataFileFormat::FbIcebergDataFileFormat_ORC:
             :               file_desc->file_format = THdfsFileFormat::ORC;
             :               break;
             :             case 
FbIcebergDataFileFormat::FbIcebergDataFileFormat_AVRO:
             :               file_desc->file_format = THdfsFileFormat::AVRO;
             :               break;
             :             default:
             :               return Status(Substitute(
             :                   "Unknown Iceberg file format type: $0",
             :                   
file_metadata->iceberg_metadata()->file_format()));
             :           }
             :         } else {
             :           file_desc->file_format = partition_desc->file_format();
             :         }
Do you think we could move this logic into FileMetadataUtils?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I941adfb659218283eb5fec1b394bb3003f8072a6
Gerrit-Change-Number: 19353
Gerrit-PatchSet: 1
Gerrit-Owner: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Gergely Fürnstáhl <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Tamas Mate <[email protected]>
Gerrit-Comment-Date: Wed, 14 Dec 2022 13:42:19 +0000
Gerrit-HasComments: Yes

Reply via email to