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: 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