diff --git a/ruby/red-arrow/ext/arrow/memory-view.cpp b/ruby/red-arrow/ext/arrow/memory-view.cpp index 3869b45583d8..27c8f1ffe1c3 100644 --- a/ruby/red-arrow/ext/arrow/memory-view.cpp +++ b/ruby/red-arrow/ext/arrow/memory-view.cpp @@ -68,19 +68,26 @@ namespace red_arrow { class PrimitiveArrayGetter : public arrow::ArrayVisitor { public: + // If view is nullptr, this only validates whether the visited + // array can be represented by memory view. explicit PrimitiveArrayGetter(memory_view *view) : view_(view) { } arrow::Status Visit(const arrow::BooleanArray& array) override { - fill(static_cast(array)); + if (array.offset() % 8 != 0) { + return arrow::Status::Invalid("Boolean array offset must be byte aligned"); + } + if (!view_) { + return arrow::Status::OK(); + } // Memory view doesn't support bit stream. We use one byte // for 8 elements. Users can't calculate the number of // elements from memory view but it's limitation of memory view. #ifdef ARROW_LITTLE_ENDIAN - view_->format = "b8"; + fill(static_cast(array), "b8"); #else - view_->format = "B8"; + fill(static_cast(array), "B8"); #endif view_->item_size = 1; view_->byte_size = (array.length() + 7) / 8; @@ -88,67 +95,59 @@ namespace red_arrow { } arrow::Status Visit(const arrow::Int8Array& array) override { - fill(static_cast(array)); - view_->format = "c"; + fill(static_cast(array), "c"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::Int16Array& array) override { - fill(static_cast(array)); - view_->format = "s"; + fill(static_cast(array), "s"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::Int32Array& array) override { - fill(static_cast(array)); - view_->format = "l"; + fill(static_cast(array), "l"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::Int64Array& array) override { - fill(static_cast(array)); - view_->format = "q"; + fill(static_cast(array), "q"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::UInt8Array& array) override { - fill(static_cast(array)); - view_->format = "C"; + fill(static_cast(array), "C"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::UInt16Array& array) override { - fill(static_cast(array)); - view_->format = "S"; + fill(static_cast(array), "S"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::UInt32Array& array) override { - fill(static_cast(array)); - view_->format = "L"; + fill(static_cast(array), "L"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::UInt64Array& array) override { - fill(static_cast(array)); - view_->format = "Q"; + fill(static_cast(array), "Q"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::FloatArray& array) override { - fill(static_cast(array)); - view_->format = "f"; + fill(static_cast(array), "f"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::DoubleArray& array) override { - fill(static_cast(array)); - view_->format = "d"; + fill(static_cast(array), "d"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::FixedSizeBinaryArray& array) override { - fill(static_cast(array)); + if (!view_) { + return arrow::Status::OK(); + } auto priv = static_cast(view_->private_data); const auto type = std::static_pointer_cast( @@ -156,60 +155,59 @@ namespace red_arrow { std::ostringstream output; output << "C" << type->byte_width(); priv->format = output.str(); - view_->format = priv->format.c_str(); + fill(static_cast(array), priv->format.c_str()); return arrow::Status::OK(); } arrow::Status Visit(const arrow::Date32Array& array) override { - fill(static_cast(array)); - view_->format = "l"; + fill(static_cast(array), "l"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::Date64Array& array) override { - fill(static_cast(array)); - view_->format = "q"; + fill(static_cast(array), "q"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::Time32Array& array) override { - fill(static_cast(array)); - view_->format = "l"; + fill(static_cast(array), "l"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::Time64Array& array) override { - fill(static_cast(array)); - view_->format = "q"; + fill(static_cast(array), "q"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::TimestampArray& array) override { - fill(static_cast(array)); - view_->format = "q"; + fill(static_cast(array), "q"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::Decimal128Array& array) override { - fill(static_cast(array)); - view_->format = "q2"; + fill(static_cast(array), "q2"); return arrow::Status::OK(); } arrow::Status Visit(const arrow::Decimal256Array& array) override { - fill(static_cast(array)); - view_->format = "q4"; + fill(static_cast(array), "q4"); return arrow::Status::OK(); } private: - void fill(const arrow::Array& array) { + void fill(const arrow::Array& array, const char *format) { + if (!view_) { + return; + } + view_->format = format; const auto array_data = array.data(); - const auto data = array_data->GetValuesSafe(1); - view_->data = const_cast(reinterpret_cast(data)); const auto type = std::static_pointer_cast(array.type()); view_->item_size = type->bit_width() / 8; + const auto byte_offset = + array_data->offset * type->bit_width() / 8; + const auto data = array_data->GetValuesSafe(1, byte_offset); + view_->data = const_cast(reinterpret_cast(data)); view_->byte_size = view_->item_size * array.length(); } @@ -229,6 +227,8 @@ namespace red_arrow { PrimitiveArrayGetter getter(view_); auto status = arrow_array->Accept(&getter); if (!status.ok()) { + delete static_cast(view_->private_data); + view_->private_data = nullptr; return false; } view_->readonly = true; @@ -243,7 +243,10 @@ namespace red_arrow { } bool primitive_array_available_p(VALUE obj) { - return true; + auto array = GARROW_ARRAY(RVAL2GOBJ(obj)); + auto arrow_array = garrow_array_get_raw(array); + PrimitiveArrayGetter validator(nullptr); + return arrow_array->Accept(&validator).ok(); } rb_memory_view_entry_t primitive_array_entry = { diff --git a/ruby/red-arrow/test/test-memory-view.rb b/ruby/red-arrow/test/test-memory-view.rb index 8aa5728a56a4..408ff68d8f27 100644 --- a/ruby/red-arrow/test/test-memory-view.rb +++ b/ruby/red-arrow/test/test-memory-view.rb @@ -29,102 +29,226 @@ def little_endian? [1].pack("s") == [1].pack("s<") end - test("BooleanArray") do - array = Arrow::BooleanArray.new([true] * 9) - Fiddle::MemoryView.export(array) do |memory_view| + def memory_view_available?(target) + libruby = Fiddle.dlopen(nil) + rb_memory_view_available_p = Fiddle::Function.new( + libruby["rb_memory_view_available_p"], + [Fiddle::TYPE_UINTPTR_T], + Fiddle::TYPE_BOOL + ) + rb_memory_view_available_p.call(Fiddle.dlwrap(target)) + end + + sub_test_case("BooleanArray") do + def template if little_endian? - template = "b" + "b" else - template = "B" + "B" + end + end + + test("offset: 0") do + array = Arrow::BooleanArray.new([true] * 9) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "#{template}8", + 1, + 2, + [(("1" * 9) + ("0" * 7))].pack("#{template}*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end + end + + test("offset: byte aligned") do + array = Arrow::BooleanArray.new(([false] * 8) + ([true] * 9)).slice(8, 9) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "#{template}8", + 1, + 2, + [("1" * 9) + ("0" * 7)].pack("#{template}*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end + end + + test("offset: not byte aligned") do + array = Arrow::BooleanArray.new([false, true, false]).slice(1, 2) + assert do + not memory_view_available?(array) end - assert_equal([ - "#{template}8", - 1, - 2, - [(("1" * 9) + ("0" * 7))].pack("#{template}*"), - ], - [ - memory_view.format, - memory_view.item_size, - memory_view.byte_size, - memory_view.to_s, - ]) end end - test("Int8Array") do - values = [-(2 ** 7), 0, (2 ** 7) - 1] - array = Arrow::Int8Array.new(values) - Fiddle::MemoryView.export(array) do |memory_view| - assert_equal([ - "c", - 1, - values.size, - values.pack("c*"), - ], - [ - memory_view.format, - memory_view.item_size, - memory_view.byte_size, - memory_view.to_s, - ]) + sub_test_case("Int8Array") do + test("offset: 0") do + values = [-(2 ** 7), 0, (2 ** 7) - 1] + array = Arrow::Int8Array.new(values) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "c", + 1, + values.size, + values.pack("c*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end + end + + test("offset: non-0") do + values = [0, 1, 2] + array = Arrow::Int8Array.new(values).slice(1, 2) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "c", + 1, + 1 * 2, + values.slice(1, 2).pack("c*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end end end - test("Int16Array") do - values = [-(2 ** 15), 0, (2 ** 15) - 1] - array = Arrow::Int16Array.new(values) - Fiddle::MemoryView.export(array) do |memory_view| - assert_equal([ - "s", - 2, - 2 * values.size, - values.pack("s*"), - ], - [ - memory_view.format, - memory_view.item_size, - memory_view.byte_size, - memory_view.to_s, - ]) + sub_test_case("Int16Array") do + test("offset: 0") do + values = [-(2 ** 15), 0, (2 ** 15) - 1] + array = Arrow::Int16Array.new(values) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "s", + 2, + 2 * values.size, + values.pack("s*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end + end + + test("offset: non-0") do + values = [0, 1, 2] + array = Arrow::Int16Array.new(values).slice(1, 2) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "s", + 2, + 2 * 2, + values.slice(1, 2).pack("s*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end end end - test("Int32Array") do - values = [-(2 ** 31), 0, (2 ** 31) - 1] - array = Arrow::Int32Array.new(values) - Fiddle::MemoryView.export(array) do |memory_view| - assert_equal([ - "l", - 4, - 4 * values.size, - values.pack("l*"), - ], - [ - memory_view.format, - memory_view.item_size, - memory_view.byte_size, - memory_view.to_s, - ]) + sub_test_case("Int32Array") do + test("offset: 0") do + values = [-(2 ** 31), 0, (2 ** 31) - 1] + array = Arrow::Int32Array.new(values) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "l", + 4, + 4 * values.size, + values.pack("l*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end + end + + test("offset: non-0") do + values = [0, 1, 2] + array = Arrow::Int32Array.new(values).slice(1, 2) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "l", + 4, + 4 * 2, + values.slice(1, 2).pack("l*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end end end - test("Int64Array") do - values = [-(2 ** 63), 0, (2 ** 63) - 1] - array = Arrow::Int64Array.new(values) - Fiddle::MemoryView.export(array) do |memory_view| - assert_equal([ - "q", - 8, - 8 * values.size, - values.pack("q*"), - ], - [ - memory_view.format, - memory_view.item_size, - memory_view.byte_size, - memory_view.to_s, - ]) + sub_test_case("Int64Array") do + test("offset: 0") do + values = [-(2 ** 63), 0, (2 ** 63) - 1] + array = Arrow::Int64Array.new(values) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "q", + 8, + 8 * values.size, + values.pack("q*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end + end + + test("offset: non-0") do + values = [0, 1, 2] + array = Arrow::Int64Array.new(values).slice(1, 2) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "q", + 8, + 8 * 2, + values.slice(1, 2).pack("q*"), + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end end end @@ -242,23 +366,45 @@ def little_endian? end end - test("FixedSizeBinaryArray") do - values = ["\x01\x02", "\x03\x04", "\x05\x06"] - data_type = Arrow::FixedSizeBinaryDataType.new(2) - array = Arrow::FixedSizeBinaryArray.new(data_type, values) - Fiddle::MemoryView.export(array) do |memory_view| - assert_equal([ - "C2", - 2, - 2 * values.size, - values.join("").b, - ], - [ - memory_view.format, - memory_view.item_size, - memory_view.byte_size, - memory_view.to_s, - ]) + sub_test_case("FixedSizeBinaryArray") do + test("offset: 0") do + values = ["\x01\x02", "\x03\x04", "\x05\x06"] + data_type = Arrow::FixedSizeBinaryDataType.new(2) + array = Arrow::FixedSizeBinaryArray.new(data_type, values) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "C2", + 2, + 2 * values.size, + values.join("").b, + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end + end + + test("offset: non-0") do + values = ["\x01\x02", "\x03\x04", "\x05\x06"] + data_type = Arrow::FixedSizeBinaryDataType.new(2) + array = Arrow::FixedSizeBinaryArray.new(data_type, values).slice(1, 2) + Fiddle::MemoryView.export(array) do |memory_view| + assert_equal([ + "C2", + 2, + 2 * 2, + values.slice(1, 2).join("").b, + ], + [ + memory_view.format, + memory_view.item_size, + memory_view.byte_size, + memory_view.to_s, + ]) + end end end