Skip to content
This repository has been archived by the owner on Feb 18, 2024. It is now read-only.

MutableUtf8Array offsets bug #1256

Closed
xinnosuke opened this issue Sep 23, 2022 · 3 comments
Closed

MutableUtf8Array offsets bug #1256

xinnosuke opened this issue Sep 23, 2022 · 3 comments
Labels
bug Something isn't working no-changelog Issues whose changes are covered by a PR and thus should not be shown in the changelog

Comments

@xinnosuke
Copy link

When new MutableUtf8Array is created, it sets offsets to default 0.
But when creating Array from the MutableArray, it removes the offsets, including the initial offset 0.
And if you want to push a new value after this, it will not add back the initial offset 0, but the size of the new value.
Additionally, if the first value is a null value, it will be an error because it will try to get the last offset which is a None value.

fn test_mut_utf8() {
      let mut mut_array: MutableUtf8Array<i32> = MutableUtf8Array::new();

      for i in 0..20 {
          if i == 10 {
              let array = mut_array.as_box();

              // panics
              // mut_array.push_null();
          }
          mut_array.try_push(Some(i.to_string())).unwrap();
      }
      let array = mut_array.as_box();

      // assert fails
      assert_eq!(array.len(), 10);
  }
@jorgecarleitao
Copy link
Owner

Being fixed as part of #1260

@jorgecarleitao jorgecarleitao added the bug Something isn't working label Sep 29, 2022
@snaar
Copy link

snaar commented Oct 2, 2022

Can you possibly add something as the example test code in this issue as a unit test? I took a look at #1260 but I didn't quite see this use case tested - it's possible I just didn't read carefully enough.

@jorgecarleitao
Copy link
Owner

Here: #1266

The root cause was that as_box was std::mem::takeing the offset vec, leading to an empty offset vector (it should always contain a 0). This was causing the mutable to become invalid when re-used.

@jorgecarleitao jorgecarleitao added the no-changelog Issues whose changes are covered by a PR and thus should not be shown in the changelog label Oct 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working no-changelog Issues whose changes are covered by a PR and thus should not be shown in the changelog
Projects
None yet
Development

No branches or pull requests

3 participants