amoeba commented on code in PR #1430:
URL: https://github.com/apache/arrow-go/pull/1430#discussion_r4404136614


##########
arrow/compute/internal/kernels/vector_sort.go:
##########
@@ -161,8 +169,8 @@ func newFixedSizeBinaryComparator(chunks []arrow.Array, 
numRows int, vn bool, ro
 }
 
 // createChunkedComparator builds a column comparator for these chunks (one 
Arrow type for all chunks).
-// rowMap may be shared by aligned multi-key columns.
-func createChunkedComparator(chunks []arrow.Array, numRows int, rowMap 
logicalRowMap) (columnComparator, error) {
+// rowMap may be shared by aligned multi-key columns. floatTotalOrder selects 
IEEE 754 totalOrder for floats.

Review Comment:
   ```suggestion
   // rowMap may be shared by aligned multi-key columns. floatTotalOrder 
enables or disables IEEE 754 totalOrder for floats.
   ```
   
   nit. you may use 'selects' elsewhere, and if so feel free to ignore this



##########
arrow/compute/internal/kernels/vector_sort.go:
##########
@@ -53,11 +53,19 @@ func (SortOptions) TypeName() string { return "SortOptions" 
}
 
 type SortState = SortOptions
 
-// SortKey defines a column to sort by with its ordering and null placement 
options.
+// SortKey defines a column to sort by with its ordering, null placement and 
float ordering options.
 type SortKey struct {
        ColumnIndex   int
        Order         SortOrder
        NullPlacement NullPlacement
+       // FloatTotalOrder orders float16/float32/float64 columns by the IEEE 
754 totalOrder
+       // predicate: -NaN < -Inf < negative values < -0 < +0 < positive values 
< +Inf < +NaN.
+       // NaNs are ordered as values (not null-like) and Descending reverses 
the whole order,
+       // NaNs included; nulls still follow NullPlacement. When false (the 
default), floats
+       // compare numerically with -0 == +0 and every NaN, regardless of sign, 
is grouped next
+       // to the nulls according to NullPlacement, independent of Order (Arrow 
C++ behavior).
+       // Non-float columns ignore it.
+       FloatTotalOrder bool

Review Comment:
   Is there a performance boost/hit worth documenting?



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

Reply via email to