Skip to content

Commit

Permalink
GH-44918: [Ruby] Fix a bug that empty list is ignored in builder (#44933
Browse files Browse the repository at this point in the history
)

### Rationale for this change

`Arrow::ListArrayBuilder#append_value` with `[]` must append an empty list as an element. In general, it works but it doesn't work when list item is struct or list. Because `Arrow::{List,Struct}ArrayBuilder#append` without arguments appends an element. It's for a backward compatibility. But it has a problem for this case.

For example, the following case has this problem:

```ruby
item_type = Arrow::StructDataType.new([{name: "visible", type: :boolean}])
data_type = Arrow::ListDataType.new(name: "struct", data_type: item_type)
builder = Arrow::ListArrayBuilder.new(data_type)
builder.append_value([])
array = builder.finish
array.to_a # => must be [[]] but [] without this change
```

### What changes are included in this PR?

This should be fixed by GH-44763 but the fix was wrong. This change fixes wrong `return if` location.

### Are these changes tested?

Yes.

### Are there any user-facing changes?

Yes.
* GitHub Issue: #44918

Authored-by: Sutou Kouhei <[email protected]>
Signed-off-by: Sutou Kouhei <[email protected]>
  • Loading branch information
kou authored Dec 9, 2024
1 parent 1b3caf6 commit 1416f62
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 2 deletions.
2 changes: 1 addition & 1 deletion ruby/red-arrow/lib/arrow/list-array-builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ def append_value(*args)
when nil
append_null
when ::Array
return if value.empty?
append_value_raw
return if value.empty?
@value_builder ||= value_builder
@value_builder.append(*value)
else
Expand Down
2 changes: 1 addition & 1 deletion ruby/red-arrow/test/test-list-array-builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ def setup
builder = Arrow::ListArrayBuilder.new(data_type)
builder.append_value([])
array = builder.finish
assert_equal([], array[0].to_a)
assert_equal([[]], array.to_a)
end
end

Expand Down

0 comments on commit 1416f62

Please sign in to comment.