Skip to content

ARROW-3831: [C++] Add support for returning decompressed size#3024

Closed
kou wants to merge 4 commits into
apache:masterfrom
kou:cpp-codec-decompressed-size
Closed

ARROW-3831: [C++] Add support for returning decompressed size#3024
kou wants to merge 4 commits into
apache:masterfrom
kou:cpp-codec-decompressed-size

Conversation

@kou

@kou kou commented Nov 24, 2018

Copy link
Copy Markdown
Member

No description provided.

@wesm

wesm commented Nov 24, 2018

Copy link
Copy Markdown
Member
[ RUN      ] TestGZip/CodecTest.CodecRoundtrip/0
..\src\arrow\util\compression-test.cc(88): error:       Expected: data.size()
      Which is: 0
To be equal to: actual_decompressed_size
      Which is: -3689348814741910324
..\src\arrow\util\compression-test.cc(88): error:       Expected: data.size()
      Which is: 0
To be equal to: actual_decompressed_size
      Which is: -3689348814741910324

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #3024 into master will increase coverage by 1.08%.
The diff coverage is 90.74%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3024      +/-   ##
==========================================
+ Coverage   86.94%   88.03%   +1.08%     
==========================================
  Files         494      425      -69     
  Lines       70180    64841    -5339     
==========================================
- Hits        61018    57081    -3937     
+ Misses       9066     7760    -1306     
+ Partials       96        0      -96
Impacted Files Coverage Δ
cpp/src/arrow/util/compression_bz2.h 50% <ø> (ø) ⬆️
cpp/src/arrow/util/compression_brotli.h 50% <ø> (ø) ⬆️
cpp/src/arrow/util/compression_snappy.h 50% <ø> (ø) ⬆️
cpp/src/arrow/util/compression.h 100% <ø> (ø) ⬆️
cpp/src/arrow/util/compression_lz4.h 50% <ø> (ø) ⬆️
cpp/src/arrow/util/compression_zstd.h 50% <ø> (ø) ⬆️
cpp/src/arrow/util/compression_bz2.cc 57.02% <0%> (-0.96%) ⬇️
cpp/src/arrow/util/compression_lz4.cc 74.78% <100%> (+0.87%) ⬆️
cpp/src/arrow/util/compression-test.cc 99.12% <100%> (+0.02%) ⬆️
cpp/src/arrow/util/compression_zstd.cc 92.07% <100%> (+0.32%) ⬆️
... and 88 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 3e0958a...9b16e58. Read the comment docs.

@kou

kou commented Nov 24, 2018

Copy link
Copy Markdown
Member Author

I've fixed it.

@wesm wesm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+1. Thanks @kou! I opened https://issues.apache.org/jira/browse/ARROW-3877 about also adding support for decompression output size hints (which are provided by some of these libraries)

@wesm wesm closed this in 1013a1d Nov 25, 2018
@kou kou deleted the cpp-codec-decompressed-size branch November 26, 2018 00:07
&decompressed_size)) {
return Status::IOError("Corrupt snappy compressed data.");
}
*output_len = static_cast<int64_t>(decompressed_size);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You don't check that output_buffer_len is large enough here?


ASSERT_EQ(data, decompressed);

// decompress with size with c2

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be useful to test what happens if the size is wrong...

@wesm

wesm commented Nov 26, 2018

Copy link
Copy Markdown
Member

@kou

kou commented Nov 30, 2018

Copy link
Copy Markdown
Member Author

I've created a pull request for it: #3057

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.

4 participants