okumin commented on code in PR #5627:
URL: https://github.com/apache/hive/pull/5627#discussion_r1952626944
##########
ql/src/test/results/clientpositive/llap/materialized_view_partitioned.q.out:
##########
@@ -1154,11 +1154,11 @@ POSTHOOK: Input: default@partition_mv_3
POSTHOOK: Input: default@partition_mv_3@key=238
#### A masked pattern was here ####
val_238_n 238
-val_238_n 238
val_238 238
val_238 238
val_238 238
val_238 238
+val_238_n 238
Review Comment:
Although this doesn't violate the semantics, I didn't understand why this
diff showed up
##########
iceberg/iceberg-handler/src/main/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandler.java:
##########
@@ -857,7 +858,12 @@ public DynamicPartitionCtx createDPContext(
if (sortField.sourceId() == field.fieldId()) {
customSortPositions.add(pos);
customSortOrder.add(sortField.direction() == SortDirection.ASC ? 1
: 0);
- customSortNullOrder.add(sortField.nullOrder() ==
NullOrder.NULLS_FIRST ? 0 : 1);
+
+ NullOrdering nullOrdering = NullOrdering.NULLS_LAST;
+ if (sortField.nullOrder() == NullOrder.NULLS_FIRST) {
+ nullOrdering = NullOrdering.NULLS_FIRST;
+ }
+ customSortNullOrder.add(nullOrdering.getCode());
Review Comment:
This seems to be OK as a conclusion, but I am leaving a note that came to my
mind.
We added a feature to configure the sort order of Iceberg tables. I wondered
if we can keep the semantics of `WRITE LOCALLY ORDERED BY a`.
https://github.com/apache/hive/pull/5541/files#r1894857758
The null ordering of sort order specs is required. So, our DDL specifies
either NULLS_FIRST or NULLS_LAST, and then `createDPContext` just follows the
definition. In other words, `sortField.nullOrder()` is non-null and this change
never takes effect in the real world.
https://github.com/apache/iceberg/blob/8839c9bf1f1d8c9b718f9766302ff8a2018e515f/core/src/main/java/org/apache/iceberg/SortOrderParser.java#L159-L175
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
