Jefffrey commented on code in PR #6321:
URL: https://github.com/apache/arrow-rs/pull/6321#discussion_r1778065451
##########
arrow-ipc/src/writer.rs:
##########
@@ -879,20 +879,31 @@ impl<W: Write> FileWriter<W> {
write_options: IpcWriteOptions,
) -> Result<Self, ArrowError> {
let data_gen = IpcDataGenerator::default();
- // write magic to header aligned on alignment boundary
- let pad_len = pad_to_alignment(write_options.alignment,
super::ARROW_MAGIC.len());
- let header_size = super::ARROW_MAGIC.len() + pad_len;
+
+ let mut written_len = 0;
+
+ // write magic and padding
writer.write_all(&super::ARROW_MAGIC)?;
+ written_len += super::ARROW_MAGIC.len();
+ let pad_len = pad_to_alignment(8, written_len);
writer.write_all(&PADDING[..pad_len])?;
- // write the schema, set the written bytes to the schema + header
+ written_len += pad_len;
+
+ // write the schema
let encoded_message = data_gen.schema_to_bytes(schema, &write_options);
- let (meta, data) = write_message(&mut writer, encoded_message,
&write_options)?;
+
+ let (meta, _data) =
+ write_message(&mut writer, written_len, encoded_message,
&write_options)?;
Review Comment:
Thoughts on putting a debug assert here for `_data == 0` to clarify why it
isn't added to `written_len`?
##########
arrow-ipc/src/writer.rs:
##########
@@ -1222,48 +1259,50 @@ pub struct EncodedData {
/// Write a message's IPC data and buffers, returning metadata and buffer data
lengths written
pub fn write_message<W: Write>(
mut writer: W,
+ already_written_len: usize,
encoded: EncodedData,
write_options: &IpcWriteOptions,
) -> Result<(usize, usize), ArrowError> {
Review Comment:
I think technically this would be a breaking change as the method is
publicly exposed to the API. Could maybe deprecate this and create it as a new
method?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]