Noemi Pap-Takacs 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 2:

(2 comments)

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) {
> We could add a DCHECK here too like L894, in case if we change/extend the f
Done


http://gerrit.cloudera.org:8080/#/c/19353/1/be/src/exec/hdfs-scan-node-base.cc@306
PS1, Line 306:         if (file_metadata) {
             :           DCHECK(file_metadata->iceberg_metadata() != nullptr);
             :           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 {
             :
> Do you think we could move this logic into FileMetadataUtils?
Makes sense, but requires a bit more refactoring. I would address this later, 
maybe together with some other scan node changes, if that is OK.



--
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: 2
Gerrit-Owner: Noemi Pap-Takacs <npaptak...@cloudera.com>
Gerrit-Reviewer: Gergely Fürnstáhl <gfurnst...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Noemi Pap-Takacs <npaptak...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tma...@apache.org>
Gerrit-Comment-Date: Thu, 15 Dec 2022 10:58:44 +0000
Gerrit-HasComments: Yes

Reply via email to