You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "pitrou (via GitHub)" <gi...@apache.org> on 2025/11/24 14:52:23 UTC
[PR] GH-48238: [C++] Actually write IPC schema endianness, not host endianness [arrow]
pitrou opened a new pull request, #48239:
URL: https://github.com/apache/arrow/pull/48239
### Rationale for this change
`Schema` objects have an endianness, but we were ignoring it serializing a `Schema` to IPC, and instead writing out the host's endianness.
### Are these changes tested?
Yes, by additional test.
### Are there any user-facing changes?
No.
**This PR contains a "Critical Fix".** (If the changes fix either (a) a security vulnerability, (b) a bug that caused incorrect or invalid data to be produced, or (c) a bug that causes a crash (even when the API contract is upheld), please provide explanation. If not, you can remove this.)
--
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
Re: [PR] GH-48238: [C++] Actually write IPC schema endianness, not host endianness [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #48239:
URL: https://github.com/apache/arrow/pull/48239#issuecomment-3584596051
> The changes look reasonable, but it would this break the "common" case if a machine is BigEndian when writing data?
What do yu mean?
--
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
Re: [PR] GH-48238: [C++] Actually write IPC schema endianness, not host endianness [arrow]
Posted by "emkornfield (via GitHub)" <gi...@apache.org>.
emkornfield commented on PR #48239:
URL: https://github.com/apache/arrow/pull/48239#issuecomment-3591697500
> > The changes look reasonable, but it would this break the "common" case if a machine is BigEndian when writing data?
>
> What do you mean?
I might have been misunderstaning the code but it looks like this PR removes automatic detection of endianness and relies the the enddianess set by the client of the library. I was wondering if this bit gets set automatically in some other way (and if not if it silently reports data from big-endian machines as little endian to consumers of IPC message.)
--
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
Re: [PR] GH-48238: [C++] Actually write IPC schema endianness, not host endianness [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #48239:
URL: https://github.com/apache/arrow/pull/48239#issuecomment-3571409424
CI failures are unrelated, some of them due to intermittent download failures on the GH infrastructure.
--
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
Re: [PR] GH-48238: [C++] Actually write IPC schema endianness, not host endianness [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou merged PR #48239:
URL: https://github.com/apache/arrow/pull/48239
--
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
Re: [PR] GH-48238: [C++] Actually write IPC schema endianness, not host endianness [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #48239:
URL: https://github.com/apache/arrow/pull/48239#issuecomment-3571186119
@Vishwanatha-HD @kiszk @k8ika0s
--
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
Re: [PR] GH-48238: [C++] Actually write IPC schema endianness, not host endianness [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #48239:
URL: https://github.com/apache/arrow/pull/48239#issuecomment-3591702245
> I might have been misunderstaning the code but it looks like this PR removes automatic detection of endianness and relies the the enddianess set by the client of the library.
The endianness is still set to the machine's default in most cases. For example when calling the Schema constructor without passing the corresponding optional argument.
--
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
Re: [PR] GH-48238: [C++] Actually write IPC schema endianness, not host endianness [arrow]
Posted by "pitrou (via GitHub)" <gi...@apache.org>.
pitrou commented on PR #48239:
URL: https://github.com/apache/arrow/pull/48239#issuecomment-3584595406
> I think we should revise the "Critical Fix" part in the PR description.
Currently, if you write non-native endianness data using IPC, the endianness is stored incorrectly. This means the data will be incorrectly byte-swapped when reading.
> One unrelated question though: How is the endianness handled for an IPC between platforms having different endiannesses?
It's optionally normalized on read. Which is why it's important for the information to be recorded correctly...
https://github.com/apache/arrow/blob/fa41b2674bfab627ec1c69b4b4fb52d9caff8c15/cpp/src/arrow/ipc/options.h#L165-L175
--
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
Re: [PR] GH-48238: [C++] Actually write IPC schema endianness, not host endianness [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #48239:
URL: https://github.com/apache/arrow/pull/48239#issuecomment-3591955211
After merging your PR, Conbench analyzed the 4 benchmarking runs that have been run so far on merge-commit 91f66187db250d844f4f62d454a8e21cdf10eb75.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/56700884112) has more details.
--
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