Marcel Kornacker has posted comments on this change. Change subject: IMPALA-4734: Set parquet::RowGroup::sorting_columns ......................................................................
Patch Set 1: (6 comments) http://gerrit.cloudera.org:8080/#/c/6219/1/be/src/exec/hdfs-table-sink.h File be/src/exec/hdfs-table-sink.h: Line 268: // Stores the positions of columns in the target table that are mentioned in the 'positions' is vague http://gerrit.cloudera.org:8080/#/c/6219/1/common/thrift/DataSinks.thrift File common/thrift/DataSinks.thrift: Line 74: // Stores the positions of columns in the target table that are mentioned in the positions sounds like indices. can you point out exactly into what they index? http://gerrit.cloudera.org:8080/#/c/6219/1/fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java File fe/src/main/java/org/apache/impala/analysis/UpdateStmt.java: Line 69: ImmutableList.<Integer>of()); i'd rather pass a null here http://gerrit.cloudera.org:8080/#/c/6219/1/fe/src/main/java/org/apache/impala/planner/Planner.java File fe/src/main/java/org/apache/impala/planner/Planner.java: Line 551: List<Boolean> isAscOrder = Collections.nCopies(orderingExprs.size(), true); also mention in commit msg that you're changing sort direction http://gerrit.cloudera.org:8080/#/c/6219/1/fe/src/main/java/org/apache/impala/planner/TableSink.java File fe/src/main/java/org/apache/impala/planner/TableSink.java: Line 89: * don't make sense for a certain table type. comment on new parameter (can it be null, etc.) Line 94: if (table instanceof HdfsTable) { for the other table types, you should check that sortbycols isn't set -- To view, visit http://gerrit.cloudera.org:8080/6219 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ib42aab585e9e627796e9510e783652d49d74b56c Gerrit-PatchSet: 1 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Lars Volker <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-HasComments: Yes
