Skip to content

throw when integer overflow occurs#8710

Merged
snleee merged 1 commit intoapache:masterfrom
richardstartin:varbytechunkwriter-overflow
May 16, 2022
Merged

throw when integer overflow occurs#8710
snleee merged 1 commit intoapache:masterfrom
richardstartin:varbytechunkwriter-overflow

Conversation

@richardstartin
Copy link
Copy Markdown
Member

Fixes #8701 by throwing early. The workaround is to start using V3 or V4.

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #8710 (d500482) into master (d3c97d5) will decrease coverage by 55.35%.
The diff coverage is 0.00%.

@@              Coverage Diff              @@
##             master    #8710       +/-   ##
=============================================
- Coverage     69.67%   14.31%   -55.36%     
+ Complexity     4571      168     -4403     
=============================================
  Files          1722     1677       -45     
  Lines         89862    87930     -1932     
  Branches      13335    13125      -210     
=============================================
- Hits          62610    12589    -50021     
- Misses        22920    74410    +51490     
+ Partials       4332      931     -3401     
Flag Coverage Δ
integration1 ?
integration2 ?
unittests1 ?
unittests2 14.31% <0.00%> (-0.02%) ⬇️

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

Impacted Files Coverage Δ
.../io/writer/impl/BaseChunkSVForwardIndexWriter.java 0.00% <0.00%> (-86.21%) ⬇️
...src/main/java/org/apache/pinot/sql/FilterKind.java 0.00% <0.00%> (-100.00%) ⬇️
...ain/java/org/apache/pinot/core/data/table/Key.java 0.00% <0.00%> (-100.00%) ⬇️
...in/java/org/apache/pinot/spi/utils/StringUtil.java 0.00% <0.00%> (-100.00%) ⬇️
.../java/org/apache/pinot/spi/utils/BooleanUtils.java 0.00% <0.00%> (-100.00%) ⬇️
.../java/org/apache/pinot/core/data/table/Record.java 0.00% <0.00%> (-100.00%) ⬇️
.../java/org/apache/pinot/core/util/GroupByUtils.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/BaseRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...java/org/apache/pinot/spi/trace/NoOpRecording.java 0.00% <0.00%> (-100.00%) ⬇️
...ava/org/apache/pinot/spi/config/table/FSTType.java 0.00% <0.00%> (-100.00%) ⬇️
... and 1342 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d3c97d5...d500482. Read the comment docs.

Copy link
Copy Markdown
Contributor

@snleee snleee left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the quick fix on this :)

@jtao15
Copy link
Copy Markdown
Contributor

jtao15 commented May 16, 2022

LGTM. Thanks for fixing this!

@snleee snleee merged commit e4a8e6e into apache:master May 16, 2022
@richardstartin
Copy link
Copy Markdown
Member Author

Just for posterity, Pinot raw forward indexes have always had this limitation; it wasn’t a regression.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

BaseChunkSVForwardIndexWriter's default version can end up storing the overflowed chunkOffset in the header

4 participants