Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/19138 )
Change subject: IMPALA-11655: Impala should set write mode "merge-on-read" by default ...................................................................... Patch Set 2: (3 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/19138/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java: http://gerrit.cloudera.org:8080/#/c/19138/1/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@658 PS1, Line 658: mat > nit: maybe make this a static final String in IcebergTable.java, so we can Done http://gerrit.cloudera.org:8080/#/c/19138/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java File fe/src/main/java/org/apache/impala/util/IcebergUtil.java: http://gerrit.cloudera.org:8080/#/c/19138/1/fe/src/main/java/org/apache/impala/util/IcebergUtil.java@839 PS1, Line 839: isAnyWriteModeSet > nit: Result from this method is always negated in all invocation. Maybe con I prefer not to put negations in method names, but it's more of a matter of taste I guess. I can change if you feel strong about it. http://gerrit.cloudera.org:8080/#/c/19138/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test: http://gerrit.cloudera.org:8080/#/c/19138/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-alter.test@354 PS1, Line 354: ---- QUERY : CREATE TABLE iceberg_upgrade_v2_no_write_mode (i INT) STORED AS ICEBERG; : DESCRIBE FORMATTED iceberg_upgrade_v2_no_write_mode; : ---- RESULTS: VERIFY_IS_NOT_IN : '','write.delete.mode ','merge-on-read ' : '','write.update.mode ','merge-on-read ' : '','write.merge.mode ','merge-on-read ' > nit: Add one more test where it explicitly set 'format-version'='1'? Done. Also added one in show-create-table.test. -- To view, visit http://gerrit.cloudera.org:8080/19138 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Icaa32472cde98e21fb23f5461175db1bf401db3d Gerrit-Change-Number: 19138 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Riza Suminto <riza.sumi...@cloudera.com> Gerrit-Reviewer: Tamas Mate <tma...@apache.org> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Mon, 17 Oct 2022 14:27:39 +0000 Gerrit-HasComments: Yes
