-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Go] ValueOffset of array.Binary panics on last index #38458
Comments
@frbvianna did you get answer regards to this issue? I have the same error. |
@polestar1988 No answers yet, but validated in local code that the solution is indeed to change |
I also handled it in my code instead of using func (a *Binary) ValueOffset(i int) :
|
Thanks @polestar1988, that is one alternative. In my case (#37976 (comment)), I was trying to make use of |
So, at minimum this should be fixed to be consistent between the That all said: I believe the intent of the If you're trying to get the element size, there's already a So, I believe the optimal solution here would be to expose the What do you both think?
This was done for consistency as most of the existing row index params in the array types use |
As long as it is ensured to work for the last possible offset / record index, this seems like a reasonable solution to me. It would simplify things for us as well. |
Just to note here, using |
Sorry for the delay here, I've been at a conference all last week, and an off-site for the week before that.
This doesn't quite make sense to me. Am I missing something? Can you provide some sample code that calls Otherwise, I'm gonna put together a PR to add |
@zeroshade, sorry for responding so late, I just checked again that I was mistaken and probably tried to use the |
### Rationale for this change Adding `ValueLen` to the `BinaryLike` interface for easy convenience of determining the length of an individual value for a Binary/String like array. ### Are these changes tested? yes * Closes: #38458 Authored-by: Matt Topol <zotthewizard@gmail.com> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
### Rationale for this change Adding `ValueLen` to the `BinaryLike` interface for easy convenience of determining the length of an individual value for a Binary/String like array. ### Are these changes tested? yes * Closes: apache#38458 Authored-by: Matt Topol <zotthewizard@gmail.com> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
### Rationale for this change Adding `ValueLen` to the `BinaryLike` interface for easy convenience of determining the length of an individual value for a Binary/String like array. ### Are these changes tested? yes * Closes: apache#38458 Authored-by: Matt Topol <zotthewizard@gmail.com> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
### Rationale for this change Adding `ValueLen` to the `BinaryLike` interface for easy convenience of determining the length of an individual value for a Binary/String like array. ### Are these changes tested? yes * Closes: apache#38458 Authored-by: Matt Topol <zotthewizard@gmail.com> Signed-off-by: Matt Topol <zotthewizard@gmail.com>
Describe the bug, including details regarding any error messages, version, and platform.
As detailed in #37976 (comment), we are using ValueOffset64 method of binary-like variable width types (string, binary) to calculate the offset differences and determine all individual value byte widths.
For an Arrow Record of e.g. 10 rows with an
array.Binary
field, a panic will occur onValueOffset(10)
(last index, matching the data slice length):arrow/go/arrow/array/binary.go
Lines 76 to 78 in f0c8229
This seems to be due to use of
>=
instead of>
. Apparently the same applies forarray.LargeBinary
fields.For comparison,
array.String
fields do not panic when the index input matches the data length:arrow/go/arrow/array/string.go
Lines 66 to 68 in f0c8229
Component(s)
Go
The text was updated successfully, but these errors were encountered: