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

Add schema as input to the decoder. #12981

Merged
merged 4 commits into from
Apr 24, 2024

Conversation

rseetham
Copy link
Contributor

resolves #12521

@rseetham
Copy link
Contributor Author

@Jackie-Jiang Addressed the comment in #12813. Had to close that pull request due to some issues with my local branch.

deprecated the no schema init and make the schema init throw an exception by default which is handled in StreamDataProvoder

@codecov-commenter
Copy link

codecov-commenter commented Apr 22, 2024

Codecov Report

Attention: Patch coverage is 72.72727% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 62.17%. Comparing base (59551e4) to head (eb5b5ab).
Report is 361 commits behind head on master.

Files Patch % Lines
...a/manager/realtime/RealtimeSegmentDataManager.java 75.00% 2 Missing ⚠️
.../apache/pinot/spi/stream/StreamMessageDecoder.java 66.66% 1 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master   #12981      +/-   ##
============================================
+ Coverage     61.75%   62.17%   +0.42%     
+ Complexity      207      198       -9     
============================================
  Files          2436     2502      +66     
  Lines        133233   136586    +3353     
  Branches      20636    21145     +509     
============================================
+ Hits          82274    84928    +2654     
- Misses        44911    45382     +471     
- Partials       6048     6276     +228     
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 35.12% <72.72%> (-26.59%) ⬇️
java-21 62.06% <72.72%> (+0.44%) ⬆️
skip-bytebuffers-false 62.14% <72.72%> (+0.39%) ⬆️
skip-bytebuffers-true 62.04% <72.72%> (+34.31%) ⬆️
temurin 62.17% <72.72%> (+0.42%) ⬆️
unittests 62.17% <72.72%> (+0.42%) ⬆️
unittests1 46.74% <72.72%> (-0.15%) ⬇️
unittests2 27.94% <0.00%> (+0.21%) ⬆️

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.

@Jackie-Jiang Jackie-Jiang added Configuration Config changes (addition/deletion/change in behavior) ingestion enhancement real-time and removed Configuration Config changes (addition/deletion/change in behavior) labels Apr 22, 2024
@rseetham rseetham force-pushed the add_schema_decoder branch 5 times, most recently from f59bbaf to a0cf980 Compare April 24, 2024 06:06
Comment on lines 1795 to 1800
try {
decoder.init(decoderProperties, fieldsToRead, _streamConfig.getTopicName());
} catch (UnsupportedOperationException e) {
// Backward compatibility
decoder.init(fieldsToRead, _streamConfig, _tableConfig, _schema);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the default fallback in the interface, we should just invoke the new method

@Jackie-Jiang Jackie-Jiang merged commit 099a86c into apache:master Apr 24, 2024
20 checks passed
@rseetham rseetham deleted the add_schema_decoder branch April 24, 2024 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Provide pinot schema when initializing StreamMessageDecoder
3 participants