Amos Bird has posted comments on this change. Change subject: IMPALA-1654: Support general predicates in most partition DDL operations. ......................................................................
Patch Set 9: (14 comments) http://gerrit.cloudera.org:8080/#/c/1563/9/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java: Line 336: TResultRow resultRow = new TResultRow(); > Using TResultRowBuilder might save a few lines. Done Line 337: Reference<Long> updated = new Reference<Long>(0L); > numUpdatedPartitions Done Line 433: resultColVal.setString_val("Updated 1 table."); > How about just "Updated table." Done Line 453: if (params.getSet_tbl_properties_params().isSetPartition_ids()) { > can you reverse this check to make the logic consistent with the one above Done Line 462: Reference<Long> col = new Reference<Long>(0L); > numUpdatedCols Done Line 598: TDdlExecResponse resp, Reference<Long> part, Reference<Long> col) throws ImpalaException { > part -> numUpdatedPartitions Done Line 665: // Set the results to be reported to the client. > This code should be unnecessary now. Remove. Done Line 1794: String.format("Ignoring empty partition list when dropping partition from " + > partitions Done Line 1816: msClient.getHiveClient().dropPartition( > Let's switch to the bulk dropPartitions() API. Be sure to honor CatalogOpEx The dropPartitions API in hmsc needs a list of ObjectPair<Integer, byte[]> which seems hard to construct in the frontend. Should I still use this? Line 1820: dropped += 1; > ++dropped; Done Line 1962: applyAlterPartition(tbl, partition); > let's update the partitions in bulk and not one at a time, take a look at b Done Line 2029: for(HdfsPartition partition: partitions) { > update in bulk, look at bulkAlterPartitions() Done Line 2240: for (HdfsPartition partition : partitions) { > update in bulk, see bulkAlterPartitions() Done Line 2245: updated += 1; > ++updated; Done -- To view, visit http://gerrit.cloudera.org:8080/1563 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f Gerrit-PatchSet: 9 Gerrit-Project: Impala Gerrit-Branch: cdh5-trunk Gerrit-Owner: Amos Bird <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Amos Bird <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Jim Apple <[email protected]> Gerrit-HasComments: Yes
