-
Notifications
You must be signed in to change notification settings - Fork 321
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
Adjusting First and Last order_by to use Sort_Column_Selector #3517
Adjusting First and Last order_by to use Sort_Column_Selector #3517
Conversation
233f128
to
1e6f45b
Compare
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.
Generally looks good, I'm just worried about violating the type signature of generate_expression
.
At least we need to update the type signature to reflect what it really can return, as otherwise it may be misleading (which is worse than having no signature at all).
However, ideally, I'd consider if we can brainstorm some solution that will be safer for future changes; ideally making the signature of this function more uniform.
distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Aggregate_Column_Helper.enso
Show resolved
Hide resolved
distribution/lib/Standard/Table/0.0.0-dev/src/Internal/Aggregate_Column_Helper.enso
Outdated
Show resolved
Hide resolved
distribution/lib/Standard/Database/0.0.0-dev/src/Data/Internal/Base_Generator.enso
Outdated
Show resolved
Hide resolved
…te_Column_Helper.enso Co-authored-by: Radosław Waśko <radoslaw.wasko@enso.org>
grouped = table.aggregate [First "TextWithNothing" (order_by = Sort_Column_Selector.By_Name [Sort_Column.Name "Value" Sort_Direction.Descending, Sort_Column.Name "Flag"]), Last "ValueWithNothing" (order_by = Sort_Column_Selector.By_Name [Sort_Column.Name "Value" Sort_Direction.Descending])] | ||
grouped = table.aggregate [First "TextWithNothing" (order_by = Sort_Column_Selector.By_Name [Sort_Column.Name "Value" Sort_Direction.Descending, Sort_Column.Name "Code"]), Last "ValueWithNothing" (order_by = Sort_Column_Selector.By_Name [Sort_Column.Name "Value" Sort_Direction.Descending])] |
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.
Don't you think it may be worth it to have this paired test as I suggested?
Pull Request Description
Important Notes
Checklist
Please include the following checklist in your PR:
Scala,
Java,
and
Rust
style guides.
./run ide dist
and./run ide watch
.