Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/17207 )
Change subject: IMPALA-10597: Enable setting 'iceberg.file_format' ...................................................................... Patch Set 4: (2 comments) http://gerrit.cloudera.org:8080/#/c/17207/3/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java File fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java: http://gerrit.cloudera.org:8080/#/c/17207/3/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@171 PS3, Line 171: Preconditions.checkState(fileformat != null); > Could you please add a test for this as well? Done http://gerrit.cloudera.org:8080/#/c/17207/3/fe/src/main/java/org/apache/impala/analysis/AlterTableSetTblProperties.java@182 PS3, Line 182: "file %s with file format %s."; : if (!firstFile.format().name().equalsIgnoreCase(fileformat)) { : throw new AnalysisException(String.format(errorMsg, fileformat, firstFile.path(), : > Is this part needed? The for loop below would cover the same, right? Here we check if the first file's format equals to the parameter 'fileformat'. The below for loop checks if every file format in 'dataFiles' are the same. If not, it prints out the first file format that differs from the others, and therefore differs from the parameter 'fileformat' as well. If we remove this 'if', then the user could set the file format to PARQUET even if all the data files are ORC. -- To view, visit http://gerrit.cloudera.org:8080/17207 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I4b3506be4562a1ace3e6435867aadb3bdde7a8e2 Gerrit-Change-Number: 17207 Gerrit-PatchSet: 4 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: 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: Mon, 29 Mar 2021 11:30:38 +0000 Gerrit-HasComments: Yes
