Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16825 )
Change subject: IMPALA-10380: INSERT INTO Iceberg tables with 'IDENTITY' partitions only ...................................................................... Patch Set 12: (7 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/16825/11//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16825/11//COMMIT_MSG@35 PS11, Line 35: TODO: : * Current change includes some parts of IMPALA-10384 which needs to : be removed once https://gerrit.cloudera.org/#/c/16850/ is merged > https://gerrit.cloudera.org/#/c/16850/ is already merged Done http://gerrit.cloudera.org:8080/#/c/16825/12//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/16825/12//COMMIT_MSG@12 PS12, Line 12: Identity-partitioned Iceberg tables are similar to regular : partitioned tables > Can you mention the difference in INSERT syntax? I mean that there is no PA Done http://gerrit.cloudera.org:8080/#/c/16825/11/be/src/exec/hdfs-table-sink.cc File be/src/exec/hdfs-table-sink.cc: http://gerrit.cloudera.org:8080/#/c/16825/11/be/src/exec/hdfs-table-sink.cc@157 PS11, Line 157: // For Iceberg tables we only have a single partition descriptor. : if (IsIceberg()) break; > This is the same as skipping the whole for loop, right? I would prefer to p Extracted the loop to a separate function. http://gerrit.cloudera.org:8080/#/c/16825/11/be/src/exec/hdfs-table-sink.cc@579 PS11, Line 579: it != partition_descriptor_map_.end() > It it possible for this to succeed if IsIceberg() is true? If not, then I t Done http://gerrit.cloudera.org:8080/#/c/16825/11/be/src/runtime/descriptors.cc File be/src/runtime/descriptors.cc: http://gerrit.cloudera.org:8080/#/c/16825/11/be/src/runtime/descriptors.cc@250 PS11, Line 250: TIcebergPartitionSpec > Maybe const ref would be better to avoid copy. Thanks for catching this. Some programming in Java and Python and I surely forget these peculiarities. http://gerrit.cloudera.org:8080/#/c/16825/11/testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test: http://gerrit.cloudera.org:8080/#/c/16825/11/testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test@4 PS11, Line 4: partitioend > typo Done http://gerrit.cloudera.org:8080/#/c/16825/12/testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test: http://gerrit.cloudera.org:8080/#/c/16825/12/testdata/workloads/functional-query/queries/QueryTest/iceberg-partitioned-insert.test@128 PS12, Line 128: select id, bool_col, int_col, bigint_col, float_col, double_col, > Can you add test here or in iceberg-negative.test where the insert is rejec Done -- To view, visit http://gerrit.cloudera.org:8080/16825 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: If98797a2bfdc038d0467c8f83aadf1a12e1d69d4 Gerrit-Change-Number: 16825 Gerrit-PatchSet: 12 Gerrit-Owner: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Reviewer: wangsheng <sky...@163.com> Gerrit-Comment-Date: Wed, 16 Dec 2020 16:59:35 +0000 Gerrit-HasComments: Yes
