okumin commented on code in PR #5627:
URL: https://github.com/apache/hive/pull/5627#discussion_r1954465171
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/SortedDynPartitionOptimizer.java:
##########
@@ -635,34 +638,18 @@ public ReduceSinkOperator getReduceSinkOp(List<Integer>
partitionPositions, List
}
}
- // if partition and bucket columns are sorted in ascending order, by
default
- // nulls come first; otherwise nulls come last
- Integer nullOrder = order == 1 ? 0 : 1;
+ char nullOrder = NullOrdering.defaultNullOrder(order,
parseCtx.getConf()).getSign();
if (sortNullOrder != null && !sortNullOrder.isEmpty()) {
- if (sortNullOrder.get(0) == 0) {
- nullOrder = 0;
- } else {
- nullOrder = 1;
- }
- }
-
- for (Integer ignored : keyColsPosInVal) {
- newSortNullOrder.add(nullOrder);
+ nullOrder = NullOrdering.fromCode(sortNullOrder.get(0)).getSign();
}
+ StringBuilder nullOrderStr = new
StringBuilder(StringUtils.repeat(nullOrder, keyColsPosInVal.size()));
if (customSortExprPresent) {
for (int i = 0; i < customSortExprs.size() -
customSortNullOrder.size(); i++) {
- newSortNullOrder.add(nullOrder);
+ nullOrderStr.append(nullOrder);
}
- newSortNullOrder.addAll(customSortNullOrder);
- }
-
- String nullOrderStr = "";
- for (Integer i : newSortNullOrder) {
- if (i == 0) {
- nullOrderStr += "a";
- } else {
- nullOrderStr += "z";
Review Comment:
Just my note. `HiveIcebergStorageHandler` mapped NULLS_FIRST to 0 and
NULLS_LAST to 1. It should have been inverted for the consistency with
`NullOrdering#getCode`. But the inconsistency didn't introduce real issues
because this part didn't also follow `NullOrdering#getCode`.
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/SortedDynPartitionOptimizer.java:
##########
@@ -635,34 +638,18 @@ public ReduceSinkOperator getReduceSinkOp(List<Integer>
partitionPositions, List
}
}
- // if partition and bucket columns are sorted in ascending order, by
default
- // nulls come first; otherwise nulls come last
- Integer nullOrder = order == 1 ? 0 : 1;
+ char nullOrder = NullOrdering.defaultNullOrder(order,
parseCtx.getConf()).getSign();
if (sortNullOrder != null && !sortNullOrder.isEmpty()) {
- if (sortNullOrder.get(0) == 0) {
- nullOrder = 0;
- } else {
- nullOrder = 1;
- }
- }
-
- for (Integer ignored : keyColsPosInVal) {
- newSortNullOrder.add(nullOrder);
+ nullOrder = NullOrdering.fromCode(sortNullOrder.get(0)).getSign();
Review Comment:
I wondered why we pick up only the first element of `sortOrder` and
`sortNullOrder`. This question doesn't directly relate to the problem of this
ticket
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
