-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Support MAP type in derived column creation during segment reload #17113
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
Support MAP type in derived column creation during segment reload #17113
Conversation
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.
Pull Request Overview
This pull request adds support for derived MAP type columns during segment reload. The change enables creation of MAP columns from existing JSON or STRING fields through function transformations, with proper handling during table/segment reload.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| ExpressionTransformerTest.java | Adds test for JSON-to-MAP transformation and updates assertions to use assertNull |
| BaseDefaultColumnHandler.java | Implements MAP column handling in derived column creation with proper stats collection and forward index creation |
| MapColumnPreIndexStatsCollector.java | Enhances MAP stats collection to handle various value types (boolean, object, etc.) and convert them to appropriate storage types |
| PinotDataType.java | Adds BYTES conversion support and serialization/deserialization methods for MAP type |
| FunctionUtils.java | Adds MAP type detection for Map class instances |
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Show resolved
Hide resolved
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
.../apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
Show resolved
Hide resolved
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
f806765 to
daa9167
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #17113 +/- ##
============================================
- Coverage 63.17% 63.17% -0.01%
Complexity 1428 1428
============================================
Files 3104 3114 +10
Lines 183488 183994 +506
Branches 28127 28196 +69
============================================
+ Hits 115916 116235 +319
- Misses 58613 58776 +163
- Partials 8959 8983 +24
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
001491b to
a85723f
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.
Pull Request Overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Show resolved
Hide resolved
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
19e3095 to
c26daa3
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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
.../apache/pinot/segment/local/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
Show resolved
Hide resolved
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
c7f323f to
7b85d24
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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
...ache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollectorTest.java
Outdated
Show resolved
Hide resolved
7b85d24 to
f608ca9
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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
...g/apache/pinot/segment/local/segment/creator/impl/stats/MapColumnPreIndexStatsCollector.java
Outdated
Show resolved
Hide resolved
f608ca9 to
23f565f
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.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Support add a derived MAP type column from existing JSON or String field.
Reload table/segment will create the new data and index
Sample derived column could be: