You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/11/22 15:27:10 UTC
[GitHub] [arrow] pitrou opened a new pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
pitrou opened a new pull request #11756:
URL: https://github.com/apache/arrow/pull/11756
Should fix the following issues (found by OSS-Fuzz):
* https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=41221
* https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=41222
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-975641387
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-977692892
Benchmark runs are scheduled for baseline = eb2fa2df6dedf2743047df9dd19d7ed4f4482c22 and contender = 587ce1dc5fcf4172591bdfdcb8882d47507d7daa. 587ce1dc5fcf4172591bdfdcb8882d47507d7daa is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/058254f918ed4dd382211f0ff3f01022...d7e61c11cef94b7f91f281ea5271d406/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d6d46b17a6c949ba825ffd31d932f0c4...e1bfe284011747278fef378ec92472d6/)
[Finished :arrow_down:0.18% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/f91a9827fe75442e9f21b2b0a8b7c6ab...3e2bf347dbfd4220b1afe2c5ac338559/)
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-976413773
@shanhuuang Other than that, does this PR look ok?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou closed pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
pitrou closed pull request #11756:
URL: https://github.com/apache/arrow/pull/11756
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-977692892
Benchmark runs are scheduled for baseline = eb2fa2df6dedf2743047df9dd19d7ed4f4482c22 and contender = 587ce1dc5fcf4172591bdfdcb8882d47507d7daa. 587ce1dc5fcf4172591bdfdcb8882d47507d7daa is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/058254f918ed4dd382211f0ff3f01022...d7e61c11cef94b7f91f281ea5271d406/)
[Failed :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d6d46b17a6c949ba825ffd31d932f0c4...e1bfe284011747278fef378ec92472d6/)
[Finished :arrow_down:0.18% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/f91a9827fe75442e9f21b2b0a8b7c6ab...3e2bf347dbfd4220b1afe2c5ac338559/)
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-977691494
I will merge then. Thank you!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] shanhuuang commented on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
shanhuuang commented on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-976300444
> @shanhuuang It looks like it would be desirable to have distinct tests for delta-encoded Parquet files with and without nulls (missing values). Would you like to do that as a separate JIRA/PR?
Yes, I'll do it during this week :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#discussion_r754389033
##########
File path: cpp/src/parquet/encoding.cc
##########
@@ -2461,31 +2464,31 @@ class DeltaByteArrayDecoder : public DecoderImpl,
typename EncodingTraits<ByteArrayType>::Accumulator* out,
int* out_num_values) {
ArrowBinaryHelper helper(out);
- ::arrow::internal::BitmapReader bit_reader(valid_bits, valid_bits_offset, num_values);
std::vector<ByteArray> values(num_values);
- int num_valid_values = GetInternal(values.data(), num_values - null_count);
+ const int num_valid_values = GetInternal(values.data(), num_values - null_count);
DCHECK_EQ(num_values - null_count, num_valid_values);
auto values_ptr = reinterpret_cast<const ByteArray*>(values.data());
int value_idx = 0;
- for (int i = 0; i < num_values; ++i) {
- bool is_valid = bit_reader.IsSet();
- bit_reader.Next();
+ RETURN_NOT_OK(VisitNullBitmapInline(
Review comment:
Note this gracefully handles the case where there is no null bitmap since there are no nulls. It may also be significantly faster ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
pitrou commented on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-975643523
@shanhuuang It looks like it would be desirable to have distinct tests for delta-encoded Parquet files with and without nulls (missing values). Would you like to do that as a separate JIRA/PR?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot commented on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
ursabot commented on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-977692892
Benchmark runs are scheduled for baseline = eb2fa2df6dedf2743047df9dd19d7ed4f4482c22 and contender = 587ce1dc5fcf4172591bdfdcb8882d47507d7daa. 587ce1dc5fcf4172591bdfdcb8882d47507d7daa is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/058254f918ed4dd382211f0ff3f01022...d7e61c11cef94b7f91f281ea5271d406/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d6d46b17a6c949ba825ffd31d932f0c4...e1bfe284011747278fef378ec92472d6/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/f91a9827fe75442e9f21b2b0a8b7c6ab...3e2bf347dbfd4220b1afe2c5ac338559/)
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ursabot edited a comment on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
ursabot edited a comment on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-977692892
Benchmark runs are scheduled for baseline = eb2fa2df6dedf2743047df9dd19d7ed4f4482c22 and contender = 587ce1dc5fcf4172591bdfdcb8882d47507d7daa. 587ce1dc5fcf4172591bdfdcb8882d47507d7daa is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/058254f918ed4dd382211f0ff3f01022...d7e61c11cef94b7f91f281ea5271d406/)
[Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d6d46b17a6c949ba825ffd31d932f0c4...e1bfe284011747278fef378ec92472d6/)
[Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/f91a9827fe75442e9f21b2b0a8b7c6ab...3e2bf347dbfd4220b1afe2c5ac338559/)
Supported benchmarks:
ursa-i9-9960x: langs = Python, R, JavaScript
ursa-thinkcentre-m75q: langs = C++, Java
ec2-t3-xlarge-us-east-2: cloud = True
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] shanhuuang commented on pull request #11756: ARROW-14792: [C++] Fix crash when reading DELTA_BYTE_ARRAY Parquet file
Posted by GitBox <gi...@apache.org>.
shanhuuang commented on pull request #11756:
URL: https://github.com/apache/arrow/pull/11756#issuecomment-977494436
> @shanhuuang Other than that, does this PR look ok?
Yes, it looks good to me. It's pretty nice to use `VisitNullBitmapInline`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org