alamb commented on code in PR #9725:
URL: https://github.com/apache/arrow-datafusion/pull/9725#discussion_r1534400422
##########
datafusion/common/src/scalar/mod.rs:
##########
@@ -4539,7 +4539,8 @@ mod tests {
// The alignment requirements differ across architectures and
// thus the size of the enum appears to as well
- assert_eq!(std::mem::size_of::<ScalarValue>(), 48);
+ // The value can be changed depending on rust version
+ assert_eq!(std::mem::size_of::<ScalarValue>(), 64);
Review Comment:
This is an interesting change -- basically it means that all `ScalarValue`
based code will potentially consume significantly more memory (as now each
ScalarValue takes 64 bytes rather than 48). Interestingly, this is the same
behavior as aarch64 in previous rust versions (you can see this test is cfg'd
off with
```rust
#[cfg(not(target_arch = "aarch64"))]
```
I'll make a follow on PR to remove this cfg
```
##########
datafusion/substrait/src/serializer.rs:
##########
@@ -27,6 +27,7 @@ use substrait::proto::Plan;
use std::fs::OpenOptions;
use std::io::{Read, Write};
+#[allow(clippy::suspicious_open_options)]
Review Comment:
Filed https://github.com/apache/arrow-datafusion/issues/9727 to track
--
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]
