starrocks
starrocks copied to clipboard
[Enhancement] Support aggregate function map_agg.
Why I'm doing:
Map_agg is an aggregate function to generate map value, which is frequently used in Trino. To support migrate from Trino to Starrocks, develop the same function in Starrocks.
Related issue #40894
What I'm doing:
I tried 3 ways to develop this function:
- Using template: key type as KT, value type as VT, in which case neither the key or value could be complex type.
- Not using template: Both key and value could be complex type, however, we need to call map_column#remove_duplicated_keys, and there is a triple loop in it, which will highly decrease the performance of map_agg.
- Using template: key type as KT, in which case the value could be complex type.
I think the 3rd one will be the best one, and the behavior is just like Trino.
What type of PR is this:
- [ ] BugFix
- [x] Feature
- [ ] Enhancement
- [ ] Refactor
- [ ] UT
- [ ] Doc
- [ ] Tool
Does this PR entail a change in behavior?
- [ ] Yes, this PR will result in a change in behavior.
- [x] No, this PR will not result in a change in behavior.
If yes, please specify the type of change:
- [ ] Interface/UI changes: syntax, type conversion, expression evaluation, display information
- [ ] Parameter changes: default values, similar parameters but with different default values
- [ ] Policy changes: use new policy to replace old one, functionality automatically enabled
- [ ] Feature removed
- [ ] Miscellaneous: upgrade & downgrade compatibility, etc.
Checklist:
- [x] I have added test cases for my bug fix or my new feature
- [x] This pr needs user documentation (for new or modified features or behaviors)
- [ ] I have added documentation for my new feature or new function
- [ ] This is a backport pr
Bugfix cherry-pick branch check:
- [x] I have checked the version labels which the pr will be auto-backported to the target branch
- [ ] 3.3
- [ ] 3.2
- [ ] 3.1
- [ ] 3.0
- [ ] 2.5
@stdpain Thanks for the review again! I have changed the code according to your review. Could you please help me review again?
@stdpain Thanks for all the reviews!
@stdpain Hi, I'm sorry that I forgot to commit a part of FE codes. Could you please review this again?
https://github.com/Mergifyio rebase main
rebase main
❌ Unable to rebase: user stdpain is unknown.
Please make sure stdpain has logged in Mergify dashboard.
https://github.com/Mergifyio rebase main
rebase main
✅ Branch has been successfully rebased
@stdpain Hello, the CI passed(The failure in admit test seems has nothing with this PR.) and I have changed the implement way of the value in map, using a column to store values instead in order to keep the reference of slices. Could you please look at this again?
Quality Gate passed
Issues
1 New issue
0 Accepted issues
Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code
[Java-Extensions Incremental Coverage Report]
:white_check_mark: pass : 0 / 0 (0%)
[FE Incremental Coverage Report]
:white_check_mark: pass : 19 / 19 (100.00%)
file detail
| path | covered_line | new_line | coverage | not_covered_line_detail | |
|---|---|---|---|---|---|
| :large_blue_circle: | com/starrocks/catalog/FunctionSet.java | 17 | 17 | 100.00% | [] |
| :large_blue_circle: | com/starrocks/sql/analyzer/PolymorphicFunctionAnalyzer.java | 2 | 2 | 100.00% | [] |
[BE Incremental Coverage Report]
:white_check_mark: pass : 70 / 84 (83.33%)
file detail
| path | covered_line | new_line | coverage | not_covered_line_detail | |
|---|---|---|---|---|---|
| :large_blue_circle: | be/src/exprs/agg/map_agg.h | 61 | 75 | 81.33% | [55, 56, 57, 58, 59, 60, 61, 62, 63, 64, 109, 110, 111, 157] |
| :large_blue_circle: | be/src/exprs/agg/factory/aggregate_resolver_avg.cpp | 9 | 9 | 100.00% | [] |
Thanks for all the reviews!