Skip to content
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

length on slices with null doesn't work #744

Closed
bjchambers opened this issue Sep 3, 2021 · 5 comments · Fixed by #745
Closed

length on slices with null doesn't work #744

bjchambers opened this issue Sep 3, 2021 · 5 comments · Fixed by #745
Labels

Comments

@bjchambers
Copy link
Contributor

Describe the bug

length kernel (for string length) doesn't work on sliced string arrays containing null.

To Reproduce

Modify the existing tests to include null:

    /// Tests with an offset
    #[test]
    fn bit_length_offsets() -> Result<()> {
        let a = StringArray::from(vec![Some("hello"), Some(" "), Some("world"), None]);
        let b = a.slice(1, 3);
        let result = bit_length(b.as_ref())?;
        let result: &Int32Array = as_primitive_array(&result);

        let expected = Int32Array::from(vec![Some(8), Some(40), None]);
        assert_eq!(&expected, result);

        Ok(())
    }

Expected behavior
Test passes.

Actual behavior
Test does not pass.

@matthewmturner
Copy link
Contributor

@bjchambers i can try looking into this.

@bjchambers
Copy link
Contributor Author

@matthewmturner I think it should be addressed by #745.

@matthewmturner
Copy link
Contributor

@bjchambers ah missed the linked pull. thx.

@matthewmturner
Copy link
Contributor

@bjchambers out of curiousity - do you know why the length_offsets test uses bit_length function instead of length?

@bjchambers
Copy link
Contributor Author

Hmm... probably a copy-paste error when updating/adding the tests. Could you comment on the PR and I'll update/resolve when I get a chance?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants