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 2020/06/28 19:09:18 UTC
[GitHub] [arrow] wesm opened a new pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
wesm opened a new pull request #7571:
URL: https://github.com/apache/arrow/pull/7571
If the message uses V4 metadata then we also look for the "ARROW:experimental_compression" field in Message::custom_metadata so that IPC message written with 0.17.x can be read in 1.0.0 and beyond.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm commented on a change in pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
wesm commented on a change in pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#discussion_r447004437
##########
File path: cpp/src/arrow/ipc/options.h
##########
@@ -66,6 +67,10 @@ struct ARROW_EXPORT IpcWriteOptions {
/// like compression
bool use_threads = true;
+ /// \brief Format version to use for IPC messages and their
+ /// metadata. Presently using V4 version (readable by v0.8.0 and later).
+ MetadataVersion metadata_version = MetadataVersion::V4;
Review comment:
see https://issues.apache.org/jira/browse/ARROW-9265
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#issuecomment-650809427
https://issues.apache.org/jira/browse/ARROW-8671
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm commented on pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#issuecomment-650857082
This has an ASAN/UBSAN failure. I will fix within an hour
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#discussion_r447002338
##########
File path: cpp/src/arrow/ipc/options.h
##########
@@ -66,6 +67,10 @@ struct ARROW_EXPORT IpcWriteOptions {
/// like compression
bool use_threads = true;
+ /// \brief Format version to use for IPC messages and their
+ /// metadata. Presently using V4 version (readable by v0.8.0 and later).
+ MetadataVersion metadata_version = MetadataVersion::V4;
Review comment:
I find it a bit weird to expose this an option. Will we be able to write data compatible with other metadata versions?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm commented on pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#issuecomment-650829722
I confirmed that I can read compressed files (including compressed dictionaries) generated from master
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm commented on pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#issuecomment-651146389
@pitrou this is already merged (by accident actually, mistyped the PR number on the command line and went too fast), but let me know if you see anything concerning from a fuzz perspective or otherwise. I fixed one fuzz issue already in https://github.com/apache/arrow/commit/76c3e4a6d30e279fa5707f7cc14e8aacf00f08a3
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm commented on a change in pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
wesm commented on a change in pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#discussion_r447002951
##########
File path: cpp/src/arrow/ipc/options.h
##########
@@ -66,6 +67,10 @@ struct ARROW_EXPORT IpcWriteOptions {
/// like compression
bool use_threads = true;
+ /// \brief Format version to use for IPC messages and their
+ /// metadata. Presently using V4 version (readable by v0.8.0 and later).
+ MetadataVersion metadata_version = MetadataVersion::V4;
Review comment:
Yes. This is what I've been saying in the V4/V5 MetadataVersion discussion e-mail
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] pitrou commented on a change in pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
pitrou commented on a change in pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#discussion_r447003525
##########
File path: cpp/src/arrow/ipc/options.h
##########
@@ -66,6 +67,10 @@ struct ARROW_EXPORT IpcWriteOptions {
/// like compression
bool use_threads = true;
+ /// \brief Format version to use for IPC messages and their
+ /// metadata. Presently using V4 version (readable by v0.8.0 and later).
+ MetadataVersion metadata_version = MetadataVersion::V4;
Review comment:
Oops, I had misunderstood that.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm commented on pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#issuecomment-651351599
I'll close this for now. Please leave any review comments and I can address them later
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm commented on pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
wesm commented on pull request #7571:
URL: https://github.com/apache/arrow/pull/7571#issuecomment-650830163
Oops, I didn't mean to merge this patch, sorry! Please review it and I will address any code reviews as follow up
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm closed pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
wesm closed pull request #7571:
URL: https://github.com/apache/arrow/pull/7571
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] wesm closed pull request #7571: ARROW-8671: [C++] Use new BodyCompression Flatbuffers member for IPC compression metadata
Posted by GitBox <gi...@apache.org>.
wesm closed pull request #7571:
URL: https://github.com/apache/arrow/pull/7571
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org