[GitHub] [arrow] wesm commented on a change in pull request #7664: ARROW-9265: [C++] Allow writing and reading V4-compliant IPC data

[GitBox <gi...@apache.org>]

wesm commented on a change in pull request #7664:
URL: https://github.com/apache/arrow/pull/7664#discussion_r452379377



##########
File path: cpp/src/arrow/ipc/file_to_stream.cc
##########
@@ -39,7 +39,11 @@ Status ConvertToStream(const char* path) {
 
   ARROW_ASSIGN_OR_RAISE(auto in_file, io::ReadableFile::Open(path));
   ARROW_ASSIGN_OR_RAISE(auto reader, ipc::RecordBatchFileReader::Open(in_file.get()));
-  ARROW_ASSIGN_OR_RAISE(auto writer, ipc::NewStreamWriter(&sink, reader->schema()));
+  auto options = IpcWriteOptions::Defaults();
+  // Use V5 to get up-to-date Union buffer layout
+  options.metadata_version = MetadataVersion::V5;

Review comment:
       This should be the default, so not needed

##########
File path: cpp/src/arrow/ipc/stream_to_file.cc
##########
@@ -37,7 +37,10 @@ Status ConvertToFile() {
   io::StdoutStream sink;
 
   ARROW_ASSIGN_OR_RAISE(auto reader, RecordBatchStreamReader::Open(&input));
-  ARROW_ASSIGN_OR_RAISE(auto writer, NewFileWriter(&sink, reader->schema()));
+  auto options = IpcWriteOptions::Defaults();
+  // Use V5 to get up-to-date Union buffer layout
+  options.metadata_version = MetadataVersion::V5;

Review comment:
       Can remove this

##########
File path: cpp/src/arrow/testing/json_integration_test.cc
##########
@@ -83,8 +83,11 @@ static Status ConvertJsonToArrow(const std::string& json_path,
               << reader->schema()->ToString(/* show_metadata = */ true) << std::endl;
   }
 
+  auto options = IpcWriteOptions::Defaults();
+  // Use V5 to get up-to-date Union buffer layout
+  options.metadata_version = MetadataVersion::V5;

Review comment:
       Can remove

##########
File path: python/pyarrow/ipc.pxi
##########
@@ -18,6 +18,32 @@
 import warnings
 
 
+cpdef enum MetadataVersion:
+    V1 = <char> CMetadataVersion_V1
+    V2 = <char> CMetadataVersion_V2
+    V3 = <char> CMetadataVersion_V3
+    V4 = <char> CMetadataVersion_V4
+    V5 = <char> CMetadataVersion_V5
+
+
+cdef object _wrap_metadata_version(CMetadataVersion version):
+    return MetadataVersion(<char> version)

Review comment:
       Out of curiosity, where will we break (presumably earlier than this) if we were to encounter an unrecognized version?




----------------------------------------------------------------
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