HIVE-28347: Make a UDAF 'collect_set' work with complex types, even when map-side aggregation is disabled.#5323
Conversation
JeongDaeKim
changed the title
| //no map aggregation. | ||
| inputOI = parameters[0]; | ||
| return ObjectInspectorFactory.getStandardListObjectInspector( | ||
| ObjectInspectorUtils.getStandardObjectInspector(inputOI)); |
There was a problem hiding this comment.
Can we merge this case with PARTIAL1?
if (m == Mode.PARTIAL1 || m == Mode.COMPLETE) {
...
|
@JeongDaeKim Taking a glance, your finding and fix seems to be very reasonable. Could you please regenerate the result file of |
|
@okumin Thank you for a quick review! 👍 |
okumin
left a comment
okumin
left a comment
There was a problem hiding this comment.
I have done three things.
First, I checked that the evaluator is likely correctly implemented now.
Second, I checked that we have new regrettion tests. Nice work.
Third, I ran the original udaf_collect_set_2.q with set hive.map.aggr = true or false. Both generated the same result set.
Then, the changes here look to me. Note that you also need +1 from a committer.
asf-ci-hive
added
tests pending
tests passed
and removed
tests passed
tests pending
labels
|
thanks @JeongDaeKim! I'll merge it with the next green build |
|
…hen map-side aggregation is disabled (Jeongdae Kim, reviewed by Shohei Okumiya, Denys Kuzmenko) Closes apache#5323
5 participants
What changes were proposed in this pull request?
UDAFs, collect_set and collect_list, won't work with complex types, if map-side aggregation is disabled. This PR will fix this bug.
Why are the changes needed?
When users disable the map-side aggregation feature(
"hive.map.aggr), collect_set doesn't work with complex types, such as struct.Does this PR introduce any user-facing change?
No.
Is the change a dependency upgrade?
No.
How was this patch tested?
Tested with qtest (TestMiniTezCliDriver)