Copilot commented on code in PR #50452:
URL: https://github.com/apache/arrow/pull/50452#discussion_r3556158377
##########
ruby/red-arrow-format/lib/arrow-format/type.rb:
##########
@@ -715,6 +715,10 @@ def build_array(...)
def to_flatbuffers
FB::Binary::Data.new
end
+
+ def offset_pack_template
+ "l<"
+ end
Review Comment:
`offset_pack_template` is using an explicit little-endian pack directive
(`"l<"`). Other numeric types in this library use native-endian pack templates
(e.g., `Int32Type#pack_template` returns `"l"`), which stays consistent with
`IO::Buffer` typed reads/writes. Using `"l<"` here can make offsets be
interpreted incorrectly on big-endian platforms (and is inconsistent with the
rest of the type packing approach). Consider using the native template (`"l"`)
here (and likewise for the UTF8 offset template) until big-endian support is
implemented end-to-end.
##########
ruby/red-arrow-format/lib/arrow-format/type.rb:
##########
@@ -743,6 +747,10 @@ def build_array(...)
def to_flatbuffers
FB::LargeBinary::Data.new
end
+
+ def offset_pack_template
+ "q<"
+ end
Review Comment:
`offset_pack_template` uses `"q<"` (explicit little-endian), but the rest of
the type packing in this library uses native-endian templates (e.g.,
`Int64Type#pack_template` is `"q"`). Mixing these can cause offsets to be
misread via `IO::Buffer` on big-endian platforms. Consider switching to `"q"`
here as well (and applying the same change to `LargeUTF8Type`).
--
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]
