You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "hypsakata (via GitHub)" <gi...@apache.org> on 2025/12/31 06:59:51 UTC
[PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
hypsakata opened a new pull request, #48699:
URL: https://github.com/apache/arrow/pull/48699
### Rationale for this change
When building an `Arrow::Table` from a Ruby Hash passed to `Arrow::Table.new`, nested Integer arrays are incorrectly inferred as `list<uint8>` or `list<int8>` regardless of the actual values contained. Nested integer arrays should be correctly inferred as the appropriate list type (e.g., `list<int64>`, `list<uint64>`) based on their values, similar to how flat arrays are handled, unless they contain values out of range for any integer type.
### What changes are included in this PR?
This PR modifies the logic in `detect_builder_info()` to fix the inference issue. Specifically:
- **Persist `sub_builder_info` across sub-array elements**: Previously, `sub_builder_info` was recreated for each sub-array element in the Array. The logic has been updated to accumulate and carry over the builder information across elements to ensure correct type inference for the entire list.
- **Refactor Integer builder logic**: Following the pattern used for `BigDecimal`, the logic for determining the Integer builder has been moved to `create_builder()`. `detect_builder_info()` now calls this function.
**Note:**
- As a side effect of this refactoring, nested lists of `BigDecimal` (which were previously inferred as `string`) may now have their types inferred. However, comprehensive testing and verification for nested `BigDecimal` support will be addressed in a separate issue to keep this PR focused.
- We stopped using `IntArrayBuilder` for inference logic to ensure correctness. This results in a performance overhead (array building is approximately 2x slower) as we can no longer rely on the specialized builder's detection.
```text
user system total real
array_builder int32 100000 0.085867 0.000194 0.086061 ( 0.086369)
int_array_builder int32 100000 0.042163 0.001033 0.043196 ( 0.043268)
array_builder int64 100000 0.086799 0.000015 0.086814 ( 0.086828)
int_array_builder int64 100000 0.044493 0.000973 0.045466 ( 0.045469)
array_builder uint32 100000 0.085748 0.000009 0.085757 ( 0.085768)
int_array_builder uint32 100000 0.044463 0.001034 0.045497 ( 0.045498)
array_builder uint64 100000 0.084548 0.000987 0.085535 ( 0.085537)
int_array_builder uint64 100000 0.044206 0.000017 0.044223 ( 0.044225)
```
### Are these changes tested?
Yes. `ruby ruby/red-arrow/test/run-test.rb`
### Are there any user-facing changes?
Yes.
Github Issue: #48481
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "hypsakata (via GitHub)" <gi...@apache.org>.
hypsakata commented on code in PR #48699:
URL: https://github.com/apache/arrow/pull/48699#discussion_r2656149061
##########
ruby/red-arrow/lib/arrow/array-builder.rb:
##########
@@ -186,6 +199,34 @@ def create_builder(builder_info)
data_type = Decimal256DataType.new(builder_info[:precision],
builder_info[:scale])
Decimal256ArrayBuilder.new(data_type)
+ when :int
+ required_bit_length = builder_info[:bit_length] + 1
Review Comment:
It is because Ruby's `.bit_length` method returns the number of bits required to represent the integer **excluding the sign bit**.
For example, `GLib::MAXINT16` (32767) is the upper bound of `Int16`, but its `.bit_length` is 15, not 16.
```ruby
irb> GLib::MAXINT16.bit_length
=> 15
```
Although I could have adjusted the thresholds in the `if` conditions (e.g., using 15 for `Int16`), I decided to keep the `builder_info[:bit_length] + 1`. This aligns the value with standard integer type widths, which I believe makes the code more intuitive.
If necessary, I can add a comment (e.g., `# Add 1 for the sign bit`) to clarify this further.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "hypsakata (via GitHub)" <gi...@apache.org>.
hypsakata commented on code in PR #48699:
URL: https://github.com/apache/arrow/pull/48699#discussion_r2656179427
##########
ruby/red-arrow/lib/arrow/array-builder.rb:
##########
@@ -186,6 +199,34 @@ def create_builder(builder_info)
data_type = Decimal256DataType.new(builder_info[:precision],
builder_info[:scale])
Decimal256ArrayBuilder.new(data_type)
+ when :int
+ required_bit_length = builder_info[:bit_length] + 1
Review Comment:
I agree with your suggestion. Checking against defined constants (`GLib::MININT*` / `GLib::MAXINT*`) is more readable than calculating bit lengths. I will update the code.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #48699:
URL: https://github.com/apache/arrow/pull/48699#discussion_r2656162122
##########
ruby/red-arrow/lib/arrow/array-builder.rb:
##########
@@ -186,6 +199,34 @@ def create_builder(builder_info)
data_type = Decimal256DataType.new(builder_info[:precision],
builder_info[:scale])
Decimal256ArrayBuilder.new(data_type)
+ when :int
+ required_bit_length = builder_info[:bit_length] + 1
Review Comment:
Or how about computing it from `min` and `max`?
```ruby
when :int
min = builder_info[:min]
max = builder_info[:max]
if GLib::MININT8 <= min and max <= GLib::MAXINT8
Int8ArrayBuilder.new
elsif GLib::MININT16 <= min and max <= GLib::MAXINT16
Int16ArrayBuilder.new
...
else
StringArrayBuilder.new
end
end
```
##########
ruby/red-arrow/lib/arrow/array-builder.rb:
##########
@@ -186,6 +199,34 @@ def create_builder(builder_info)
data_type = Decimal256DataType.new(builder_info[:precision],
builder_info[:scale])
Decimal256ArrayBuilder.new(data_type)
+ when :int
+ required_bit_length = builder_info[:bit_length] + 1
Review Comment:
Or how about computing it from `min` and `max`?
```ruby
when :int
min = builder_info[:min]
max = builder_info[:max]
if GLib::MININT8 <= min and max <= GLib::MAXINT8
Int8ArrayBuilder.new
elsif GLib::MININT16 <= min and max <= GLib::MAXINT16
Int16ArrayBuilder.new
...
else
StringArrayBuilder.new
end
end
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #48699:
URL: https://github.com/apache/arrow/pull/48699#discussion_r2656029108
##########
ruby/red-arrow/test/test-array-builder.rb:
##########
@@ -147,44 +147,435 @@ def assert_build(builder_class, raw_array)
])
end
- test("list<uint>s") do
- values = [
- [0, 1, 2],
- [3, 4],
- ]
- array = Arrow::Array.new(values)
- data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
- assert_equal({
- data_type: data_type,
- values: [
- [0, 1, 2],
- [3, 4],
- ],
- },
- {
- data_type: array.value_data_type,
- values: array.to_a,
- })
- end
+ sub_test_case("nested integer list") do
+ test("list<uint8>s") do
+ values = [
+ [0, 1, 2],
+ [3, 4],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, 1, 2],
+ [3, 4],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
- test("list<int>s") do
- values = [
- [0, -1, 2],
- [3, 4],
- ]
- array = Arrow::Array.new(values)
- data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
- assert_equal({
- data_type: data_type,
- values: [
- [0, -1, 2],
- [3, 4],
- ],
- },
- {
- data_type: array.value_data_type,
- values: array.to_a,
- })
+ test("list<int8>s") do
+ values = [
+ [0, -1, 2],
+ [3, 4],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, -1, 2],
+ [3, 4],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
Review Comment:
Can we remove this test because `list<int8>s boundary` test covers this case?
##########
ruby/red-arrow/test/test-array-builder.rb:
##########
@@ -147,44 +147,435 @@ def assert_build(builder_class, raw_array)
])
end
- test("list<uint>s") do
- values = [
- [0, 1, 2],
- [3, 4],
- ]
- array = Arrow::Array.new(values)
- data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
- assert_equal({
- data_type: data_type,
- values: [
- [0, 1, 2],
- [3, 4],
- ],
- },
- {
- data_type: array.value_data_type,
- values: array.to_a,
- })
- end
+ sub_test_case("nested integer list") do
+ test("list<uint8>s") do
+ values = [
+ [0, 1, 2],
+ [3, 4],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, 1, 2],
+ [3, 4],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
- test("list<int>s") do
- values = [
- [0, -1, 2],
- [3, 4],
- ]
- array = Arrow::Array.new(values)
- data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
- assert_equal({
- data_type: data_type,
- values: [
- [0, -1, 2],
- [3, 4],
- ],
- },
- {
- data_type: array.value_data_type,
- values: array.to_a,
- })
+ test("list<int8>s") do
+ values = [
+ [0, -1, 2],
+ [3, 4],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, -1, 2],
+ [3, 4],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
+
+ test("list<int8>s boundary") do
+ # Int8 can hold values from -128 to 127.
+ values = [
+ [0, -2**7],
+ [2**7 - 1],
Review Comment:
How about using defined constants instead of comment?
```suggestion
values = [
[0, GLib::MININT8],
[GLib::MAXINT8],
```
##########
ruby/red-arrow/lib/arrow/array-builder.rb:
##########
@@ -186,6 +199,34 @@ def create_builder(builder_info)
data_type = Decimal256DataType.new(builder_info[:precision],
builder_info[:scale])
Decimal256ArrayBuilder.new(data_type)
+ when :int
+ required_bit_length = builder_info[:bit_length] + 1
Review Comment:
Why do we need ` +1` only for `:int`?
##########
ruby/red-arrow/test/test-array-builder.rb:
##########
@@ -147,44 +147,435 @@ def assert_build(builder_class, raw_array)
])
end
- test("list<uint>s") do
- values = [
- [0, 1, 2],
- [3, 4],
- ]
- array = Arrow::Array.new(values)
- data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
- assert_equal({
- data_type: data_type,
- values: [
- [0, 1, 2],
- [3, 4],
- ],
- },
- {
- data_type: array.value_data_type,
- values: array.to_a,
- })
- end
+ sub_test_case("nested integer list") do
+ test("list<uint8>s") do
+ values = [
+ [0, 1, 2],
+ [3, 4],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, 1, 2],
+ [3, 4],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
- test("list<int>s") do
- values = [
- [0, -1, 2],
- [3, 4],
- ]
- array = Arrow::Array.new(values)
- data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
- assert_equal({
- data_type: data_type,
- values: [
- [0, -1, 2],
- [3, 4],
- ],
- },
- {
- data_type: array.value_data_type,
- values: array.to_a,
- })
+ test("list<int8>s") do
+ values = [
+ [0, -1, 2],
+ [3, 4],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, -1, 2],
+ [3, 4],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
+
+ test("list<int8>s boundary") do
+ # Int8 can hold values from -128 to 127.
+ values = [
+ [0, -2**7],
+ [2**7 - 1],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
+
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, -128],
+ [127],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
+
+ test("list<int16>s inferred from int8 underflow") do
+ values = [
+ [0, -2**7 - 1],
+ [2**7 - 1],
Review Comment:
```suggestion
[0, GLib::MININT8 - 1],
[GLib::MAXINT8],
```
##########
ruby/red-arrow/test/test-array-builder.rb:
##########
@@ -147,44 +147,435 @@ def assert_build(builder_class, raw_array)
])
end
- test("list<uint>s") do
- values = [
- [0, 1, 2],
- [3, 4],
- ]
- array = Arrow::Array.new(values)
- data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
- assert_equal({
- data_type: data_type,
- values: [
- [0, 1, 2],
- [3, 4],
- ],
- },
- {
- data_type: array.value_data_type,
- values: array.to_a,
- })
- end
+ sub_test_case("nested integer list") do
+ test("list<uint8>s") do
+ values = [
+ [0, 1, 2],
+ [3, 4],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
Review Comment:
We can simplify this:
```suggestion
data_type = Arrow::ListDataType.new(:uint8)
```
##########
ruby/red-arrow/test/test-array-builder.rb:
##########
@@ -147,44 +147,435 @@ def assert_build(builder_class, raw_array)
])
end
- test("list<uint>s") do
- values = [
- [0, 1, 2],
- [3, 4],
- ]
- array = Arrow::Array.new(values)
- data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
- assert_equal({
- data_type: data_type,
- values: [
- [0, 1, 2],
- [3, 4],
- ],
- },
- {
- data_type: array.value_data_type,
- values: array.to_a,
- })
- end
+ sub_test_case("nested integer list") do
+ test("list<uint8>s") do
+ values = [
+ [0, 1, 2],
+ [3, 4],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::UInt8DataType.new)
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, 1, 2],
+ [3, 4],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
- test("list<int>s") do
- values = [
- [0, -1, 2],
- [3, 4],
- ]
- array = Arrow::Array.new(values)
- data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
- assert_equal({
- data_type: data_type,
- values: [
- [0, -1, 2],
- [3, 4],
- ],
- },
- {
- data_type: array.value_data_type,
- values: array.to_a,
- })
+ test("list<int8>s") do
+ values = [
+ [0, -1, 2],
+ [3, 4],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, -1, 2],
+ [3, 4],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
+
+ test("list<int8>s boundary") do
+ # Int8 can hold values from -128 to 127.
+ values = [
+ [0, -2**7],
+ [2**7 - 1],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::Int8DataType.new)
+
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, -128],
+ [127],
+ ],
+ },
+ {
+ data_type: array.value_data_type,
+ values: array.to_a,
+ })
+ end
+
+ test("list<int16>s inferred from int8 underflow") do
+ values = [
+ [0, -2**7 - 1],
+ [2**7 - 1],
+ ]
+ array = Arrow::Array.new(values)
+ data_type = Arrow::ListDataType.new(Arrow::Int16DataType.new)
+
+ # Int8 lower bound is -128
+ assert_equal({
+ data_type: data_type,
+ values: [
+ [0, -129],
+ [127],
Review Comment:
```suggestion
[0, GLib::MININT8 - 1],
[GLib::MAXINT8],
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #48699:
URL: https://github.com/apache/arrow/pull/48699#issuecomment-3706933174
After merging your PR, Conbench analyzed the 3 benchmarking runs that have been run so far on merge-commit 6ee7f7ecdc4b79b7882cdc8f47828964e3ba6a6f.
There weren't enough matching historic benchmark results to make a call on whether there were regressions.
The [full Conbench report](https://github.com/apache/arrow/runs/59362865664) has more details.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou merged PR #48699:
URL: https://github.com/apache/arrow/pull/48699
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #48699:
URL: https://github.com/apache/arrow/pull/48699#issuecomment-3701584961
:warning: GitHub issue #48481 **has been automatically assigned in GitHub** to PR creator.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "hypsakata (via GitHub)" <gi...@apache.org>.
hypsakata commented on PR #48699:
URL: https://github.com/apache/arrow/pull/48699#issuecomment-3701585841
Benchmark code
```ruby
require "benchmark"
require "arrow"
SIZES = [100_000].freeze
# Pre-generate arrays so we only measure builder cost.
DATASETS = SIZES.each_with_object({}) do |size, datasets|
datasets[["int32", size]] = Array.new(size) {|i| (i % 2_000_000) - 1_000_000 }
datasets[["int64", size]] = Array.new(size) {|i| (i % 2**40) - 2**39 }
datasets[["uint32", size]] = Array.new(size) {|i| i % 4_000_000_000 }
datasets[["uint64", size]] = Array.new(size) {|i| i % 2**40 }
end.freeze
Benchmark.bm(36) do |x|
DATASETS.each do |(name, size), values|
x.report(" array_builder #{name} #{size}") do
Arrow::ArrayBuilder.build(values)
end
next unless name.start_with?("int", "uint")
builder_class = name.start_with?("uint") ? Arrow::UIntArrayBuilder : Arrow::IntArrayBuilder
x.report("int_array_builder #{name} #{size}") do
builder = builder_class.new
builder.build(values)
end
end
end
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #48699:
URL: https://github.com/apache/arrow/pull/48699#discussion_r2656162122
##########
ruby/red-arrow/lib/arrow/array-builder.rb:
##########
@@ -186,6 +199,34 @@ def create_builder(builder_info)
data_type = Decimal256DataType.new(builder_info[:precision],
builder_info[:scale])
Decimal256ArrayBuilder.new(data_type)
+ when :int
+ required_bit_length = builder_info[:bit_length] + 1
Review Comment:
Or how about computing it from `min` and `max`?
```ruby
when :int
min = builder_info[:min]
max = builder_info[:max]
if GLib::MININT8 <= min and max <= GLib::MAXINT8
Int8ArrayBuilder.new
elsif GLib::MININT16 <= min and max <= GLib::MAXINT16
Int16ArrayBuilder.new
...
else
StringArrayBuilder.new
end
end
```
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] GH-48481: [Ruby] Correctly infer types for nested integer arrays [arrow]
Posted by "kou (via GitHub)" <gi...@apache.org>.
kou commented on code in PR #48699:
URL: https://github.com/apache/arrow/pull/48699#discussion_r2656157510
##########
ruby/red-arrow/lib/arrow/array-builder.rb:
##########
@@ -186,6 +199,34 @@ def create_builder(builder_info)
data_type = Decimal256DataType.new(builder_info[:precision],
builder_info[:scale])
Decimal256ArrayBuilder.new(data_type)
+ when :int
+ required_bit_length = builder_info[:bit_length] + 1
Review Comment:
Could you add a comment for this with an example like you showed in your reply? It may be difficult to understand why we need `+ 1` with only the `Add 1 for the sign bit` comment.
--
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: github-unsubscribe@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org