Skip to content
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 #13453

Closed
wants to merge 1 commit into from
Closed

Support map type #13453

wants to merge 1 commit into from

Conversation

xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Jun 21, 2024

@xiangfu0 xiangfu0 marked this pull request as draft June 21, 2024 07:45
@xiangfu0 xiangfu0 force-pushed the feature_map_type branch 3 times, most recently from e75d831 to 591e9d9 Compare June 21, 2024 08:32
@siddharthteotia
Copy link
Contributor

Is this about supporting complex type MAP as a first class data type in Pinot ?

I am interested in helping review this. Is there a rough design doc / high level idea that can be shared to understand the semantics of how this will look in storage and query wise ?

Are we planning to store this as a raw blob / binary bytes in forward index ?

@codecov-commenter
Copy link

codecov-commenter commented Jul 10, 2024

Codecov Report

Attention: Patch coverage is 64.88920% with 507 lines in your changes missing coverage. Please review.

Project coverage is 62.00%. Comparing base (59551e4) to head (0229ca1).
Report is 861 commits behind head on master.

Files Patch % Lines
...t/local/realtime/impl/map/MutableMapIndexImpl.java 58.01% 76 Missing and 13 partials ⚠️
...ocal/segment/creator/impl/map/MapIndexCreator.java 79.63% 45 Missing and 11 partials ⚠️
...egment/local/segment/index/map/NullDataSource.java 0.00% 41 Missing ⚠️
...or/impl/stats/MapColumnPreIndexStatsCollector.java 63.10% 29 Missing and 9 partials ⚠️
.../apache/pinot/spi/config/table/MapIndexConfig.java 0.00% 38 Missing ⚠️
...r/transform/function/MapItemTransformFunction.java 0.00% 37 Missing ⚠️
...ent/index/readers/map/ImmutableMapIndexReader.java 48.61% 34 Missing and 3 partials ⚠️
...ocal/segment/index/map/ImmutableMapDataSource.java 37.83% 22 Missing and 1 partial ⚠️
...local/segment/creator/impl/map/DenseMapHeader.java 89.14% 5 Missing and 14 partials ⚠️
...local/segment/creator/impl/map/MapIndexHeader.java 85.83% 13 Missing and 4 partials ⚠️
... and 26 more
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #13453      +/-   ##
============================================
+ Coverage     61.75%   62.00%   +0.24%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2574     +138     
  Lines        133233   142514    +9281     
  Branches      20636    22104    +1468     
============================================
+ Hits          82274    88360    +6086     
- Misses        44911    47435    +2524     
- Partials       6048     6719     +671     
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration <0.01% <0.00%> (-0.01%) ⬇️
integration1 <0.01% <0.00%> (-0.01%) ⬇️
integration2 0.00% <0.00%> (ø)
java-11 61.94% <64.88%> (+0.24%) ⬆️
java-21 61.89% <64.88%> (+0.26%) ⬆️
skip-bytebuffers-false 61.98% <64.88%> (+0.23%) ⬆️
skip-bytebuffers-true 61.85% <64.88%> (+34.12%) ⬆️
temurin 62.00% <64.88%> (+0.24%) ⬆️
unittests 61.99% <64.88%> (+0.24%) ⬆️
unittests1 45.88% <7.20%> (-1.01%) ⬇️
unittests2 28.10% <57.68%> (+0.37%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@xiangfu0 xiangfu0 force-pushed the feature_map_type branch 2 times, most recently from 00ab037 to 44f6b84 Compare July 28, 2024 06:57
@xiangfu0 xiangfu0 force-pushed the feature_map_type branch 6 times, most recently from 22e3fcf to de82015 Compare August 9, 2024 00:04
@xiangfu0 xiangfu0 marked this pull request as ready for review August 10, 2024 19:36
@xiangfu0
Copy link
Contributor Author

Is this about supporting complex type MAP as a first class data type in Pinot ?

I am interested in helping review this. Is there a rough design doc / high level idea that can be shared to understand the semantics of how this will look in storage and query wise ?

Are we planning to store this as a raw blob / binary bytes in forward index ?

Thanks Sid! Wilk share a design doc soon. The basic idea here is to store map of key to any primitive types using forward index.

And later on we can add more index support for map item function based on the density etc

@siddharthteotia
Copy link
Contributor

Sounds good. Thanks @xiangfu0 . Please share a design doc when you can.

Long back, I had done a prototype of MAP and LIST support, essentially composing these where we can compose and store primitives and complex types in a flexible way. So, I am interested.

@xiangfu0 xiangfu0 changed the title [WIP] Support map type Support map type Aug 12, 2024
@Jackie-Jiang Jackie-Jiang added feature release-notes Referenced by PRs that need attention when compiling the next release notes labels Aug 27, 2024
@xiangfu0 xiangfu0 closed this Sep 10, 2024
@xiangfu0 xiangfu0 deleted the feature_map_type branch September 10, 2024 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature release-notes Referenced by PRs that need attention when compiling the next release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants