feat: add projected_array, projected_row, field comparator, partition… - #32
Conversation
… computer and row utilities
leaves12138
left a comment
leaves12138
left a comment
There was a problem hiding this comment.
I found one correctness blocker in FieldsComparator: the FLOAT and DOUBLE comparators use plain == and <, even though CompareFloatingPoint() is already defined for Java-compatible ordering. This makes comparisons with NaN non-antisymmetric (for example, both 1.0 <=> NaN and NaN <=> 1.0 return 1), and it also treats -0.0 and +0.0 as equal despite the intended ordering documented in CompareFloatingPoint. Please route both float and double comparisons through CompareFloatingPoint() and add tests for NaN / signed-zero cases. I did not find other blockers in this pass.
leaves12138
left a comment
leaves12138
left a comment
There was a problem hiding this comment.
Re-reviewed with the clarified intended semantics. The previous blocker I raised was incorrect: FieldsComparator is intended to use PK comparator semantics where -0.0 and +0.0 compare equal and NaN is treated as equal to +infinity, while CompareFloatingPoint is for range index / SST key ordering. With that distinction, I do not find blockers in this pass.
2 participants
Purpose
Introduce core utility classes for row/array projection, field comparison, partition computation, and internal row helper functions.
Changes
ProjectedRowInternalRowimplementation that provides a projected (column-selected and reordered) view of an underlyingInternalRowProjectedArrayInternalArrayimplementation that provides a projected view of an underlyingInternalArrayFieldsComparatorInternalRowinstances field-by-field based on configurable sort fields and ordering-infinity < -0.0 < +0.0 < +infinity < NaN == NaNBinaryRowPartitionComputerBinaryRowrepresentationsPartitionConverterInternalRowUtilsInternalArrayandstd::vector<std::string>/std::vector<std::optional<std::string>>Tests
BinaryRowPartitionComputerTestFieldsComparatorTestInternalRowUtilsTestProjectedRowTestProjectedArrayTest