-
Notifications
You must be signed in to change notification settings - Fork 70
feat: sort order #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: sort order #59
Conversation
Signed-off-by: Junwang Zhao <[email protected]>
Signed-off-by: Junwang Zhao <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds functionality for sorting by implementing new SortOrder and SortField classes along with comprehensive tests.
- Implements SortOrder and SortField classes with formatting and equality comparison.
- Provides tests to validate sort order behavior and proper string output.
- Adds a custom TransformFunction for testing purposes.
Reviewed Changes
Copilot reviewed 6 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| test/sort_order_test.cc | Adds tests for basic functionality and equality for SortOrder. |
| test/sort_field_test.cc | Adds tests for basic functionality and equality for SortField. |
| src/iceberg/sort_order.h | Declares the SortOrder class with defined formatting and equality. |
| src/iceberg/sort_order.cc | Implements SortOrder with string formatting and equality check. |
| src/iceberg/sort_field.h | Declares the SortField class with string formatting and equality. |
| src/iceberg/sort_field.cc | Implements SortField with formatting and equality check. |
Files not reviewed (2)
- src/iceberg/CMakeLists.txt: Language not supported
- test/CMakeLists.txt: Language not supported
wgtmac
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Do you have any comment? @lidavidm |
No description provided.