-
Notifications
You must be signed in to change notification settings - Fork 743
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
Add checks and logic to support sort
operation for tuples at runtime
#43290
Add checks and logic to support sort
operation for tuples at runtime
#43290
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #43290 +/- ##
============================================
+ Coverage 77.49% 77.51% +0.01%
- Complexity 58576 58625 +49
============================================
Files 3438 3439 +1
Lines 219211 219299 +88
Branches 28913 28937 +24
============================================
+ Hits 169880 169986 +106
+ Misses 39914 39892 -22
- Partials 9417 9421 +4 ☔ View full report in Codecov by Sentry. |
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Sort.java
Outdated
Show resolved
Hide resolved
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/utils/SortUtils.java
Show resolved
Hide resolved
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Sort.java
Outdated
Show resolved
Hide resolved
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Sort.java
Outdated
Show resolved
Hide resolved
langlib/langlib-test/src/test/resources/test-src/arraylib_test.bal
Outdated
Show resolved
Hide resolved
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Sort.java
Outdated
Show resolved
Hide resolved
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Sort.java
Outdated
Show resolved
Hide resolved
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/utils/SortUtils.java
Show resolved
Hide resolved
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/utils/SortUtils.java
Outdated
Show resolved
Hide resolved
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/utils/SortUtils.java
Show resolved
Hide resolved
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
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/Sort.java
Outdated
Show resolved
Hide resolved
langlib/lang.array/src/main/java/org/ballerinalang/langlib/array/utils/SortUtils.java
Outdated
Show resolved
Hide resolved
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
@ravinperera00 This change can affect the order by clause in query expression as well. So shall we add few tests for sorting tuples with query expressions as well? |
Purpose
Fixes #43029
Approach
The method definition for the sort operation is as follows.
One of the major problems with enabling sorting for tuples is the handling of comparison between more than one data type during sorting. According to the definition, a key function is provided for this reason, if the members don't belong to a single orderedType.
The definition of OrderedType is as follows,
The sort operation should allow any member type to be present (regardless of whether the type belongs to an OrderedType or not) within the tuple as long as a key function is provided when needed (if all the members don't belong to a single OrderedType). Any error that is occurred during the operation will cause a panic. The runtime shall implement checks and logic need to verify if a given type is an OrderedType and whether the operation required a key function. Once the necessary conditions are met, the operation will sort the given tuple and provide a new sorted array with a union type that covers the types of the members in the tuple.
Samples
Example without a key function
Example with a key function
Remarks
Check List