You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@druid.apache.org by "clintropolis (via GitHub)" <gi...@apache.org> on 2023/02/09 11:32:14 UTC

[GitHub] [druid] clintropolis opened a new pull request, #13781: fix array_agg to work with complex types and bugs with expression aggregator complex array handling

clintropolis opened a new pull request, #13781:
URL: https://github.com/apache/druid/pull/13781

   ### Description
   This PR removes an overly restrictive validation check on `ARRAY_AGG` which was preventing use with druid `COMPLEX` types, such as `COMPLEX<json>`. This check is a relic from when this function was added, which was before the native expression system supported `COMPLEX` types.
   
   After removing this check, I ran into a couple of rough edges, and a bug in `ObjectStrategyComplexTypeStrategy`, which was missing a check to ensure that the positional `read` method actually moved the buffer position forward by the number of bytes read (useful for array_agg which is serializing arrays) instead of relying on the underlying `ObjectStrategy` to move the buffer.
   
   After the bugs, the result seems pretty useful, particularly for `COMPLEX<json>` (though certainly not the speediest thing around since its implemented as an expression aggregator):
   <img width="1239" alt="Screenshot 2023-02-09 at 3 06 39 AM" src="https://user-images.githubusercontent.com/1577461/217799893-09b463bb-6ade-4c5f-a39c-a86ad4f97d9b.png">
   even distinct works:
   
   <img width="1236" alt="Screenshot 2023-02-09 at 3 06 05 AM" src="https://user-images.githubusercontent.com/1577461/217799943-bb01eccb-8242-48ec-acf1-026ebbad3650.png">
   
   
   This PR has:
   
   - [x] been self-reviewed.
   - [ ] a release note entry in the PR description.
   - [ ] added unit tests or modified existing tests to cover new code paths, ensuring the threshold for [code coverage](https://github.com/apache/druid/blob/master/dev/code-review/code-coverage.md) is met.
   - [x] been tested in a test Druid cluster.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org


[GitHub] [druid] clintropolis merged pull request #13781: fix array_agg to work with complex types and bugs with expression aggregator complex array handling

Posted by "clintropolis (via GitHub)" <gi...@apache.org>.
clintropolis merged PR #13781:
URL: https://github.com/apache/druid/pull/13781


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org