Copilot commented on code in PR #50433:
URL: https://github.com/apache/arrow/pull/50433#discussion_r3549073387
##########
ruby/red-arrow-format/lib/arrow-format/array.rb:
##########
@@ -64,18 +66,28 @@ def empty?
@size.zero?
end
+ def ==(other)
+ return false unless other.is_a?(self.class)
+ return false unless @size == other.size
+ return true if @validity_buffer.nil? and other.validity_buffer.nil?
+ if @offset == other.offset and @validity_buffer == other.validity_buffer
+ return true
+ end
+ validity_bitmap == other.validity_bitmap
+ end
Review Comment:
`ArrowFormat::Array#==` only compares size/validity (and even returns `true`
when both `validity_buffer`s are `nil`). For non-`PrimitiveArray` subclasses
(e.g. `VariableSizeBinaryArray`, `StructArray`, `DictionaryArray`, …) this
means two arrays with different values but the same length will be considered
equal, which breaks the promised content-based comparison.
##########
ruby/red-arrow-format/lib/arrow-format/array.rb:
##########
@@ -64,18 +66,28 @@ def empty?
@size.zero?
end
+ def ==(other)
+ return false unless other.is_a?(self.class)
+ return false unless @size == other.size
Review Comment:
`ArrowFormat::Array#==` is a user-facing behavior change for many array
types beyond numeric/boolean/null. The current tests only cover primitive
arrays and `Bitmap`; adding at least one `==` test for a non-primitive array
(e.g. `BinaryArray`/`UTF8Array`, `StructArray`, or a list/dictionary array)
would help prevent regressions where only validity/length is compared.
--
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]
