Copilot commented on code in PR #50443: URL: https://github.com/apache/arrow/pull/50443#discussion_r3549860347
########## ruby/red-arrow-format/lib/arrow-format/array.rb: ########## @@ -461,9 +465,29 @@ class TimeArray < TemporalArray end class Time32Array < TimeArray + private + def ensure_type(type) + case type + when Symbol + unit = type + Time32Type.new(unit) + else + type + end + end end class Time64Array < TimeArray + private + def ensure_type(type) + case type + when Symbol + unit = type + Time64Type.new(unit) + else + type + end + end Review Comment: `Time64Array.new(:unit, values)` currently accepts any Symbol and will create unsupported combinations like `Time64Type.new(:second)`. The read-path only supports :microsecond/:nanosecond for 64-bit times, and other Arrow implementations may reject these schemas. Consider validating the unit and raising an ArgumentError for unsupported symbols. ########## ruby/red-arrow-format/lib/arrow-format/array.rb: ########## @@ -461,9 +465,29 @@ class TimeArray < TemporalArray end class Time32Array < TimeArray + private + def ensure_type(type) + case type + when Symbol + unit = type + Time32Type.new(unit) + else + type + end + end Review Comment: `Time32Array.new(:unit, values)` currently accepts any Symbol and will happily create `Time32Type.new(:microsecond)` etc. Those (bit_width=32, unit=microsecond/nanosecond) are treated as unsupported on the read-path (`readable.rb` only supports :second/:millisecond for 32-bit), and they’re not valid Arrow time32 encodings. Consider validating the unit and raising an ArgumentError for unsupported symbols. -- 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]
