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

feat: ListArray support #1080

Merged
merged 26 commits into from
Mar 21, 2022
Merged

feat: ListArray support #1080

merged 26 commits into from
Mar 21, 2022

Conversation

nanderstabel
Copy link
Contributor

@nanderstabel nanderstabel commented Mar 20, 2022

What's changed and what's your intention?

Addition of ListArray to our list of Array types (#578 and #579). The intention was to use the already existing implementation of StructArray as a foundation and rewrite it to make it fit to the requirements as described here.

Both List<T> and List<List<T>> are supported.

  • Check behavior of appending ListArrays with different nested layers
  • Check impls for ListRef (many of them are still copy-pastes from their respective StructRef counterparts)
  • Write tests for value comparison (test_list_value_cmp)

Checklist

  • I have written necessary docs and comments
  • I have added necessary unit tests and integration tests

@codecov
Copy link

codecov bot commented Mar 20, 2022

The author of this PR, nanderstabel, is not an activated member of this organization on Codecov.
Please activate this user on Codecov to display this PR comment.
Coverage data is still being uploaded to Codecov.io for purposes of overall coverage calculations.
Please don't hesitate to email us at [email protected] with any questions.

This was linked to issues Mar 20, 2022
@nanderstabel
Copy link
Contributor Author

With these latest changes this issue is ready to get merged (or else very close to being ready).

Consideration:
When declaring a new ListArrayBuilder I've used DataType::Int16 as a default datatype:

    #[cfg(test)]
    fn new(capacity: usize) -> Result<Self> {
        Self::new_with_meta(
            capacity,
            ArrayMeta::List {
                // Default datatype
                datatype: Box::new(DataType::Int16),
            },
        )
    }

This might not best solution in the future.

@nanderstabel nanderstabel marked this pull request as ready for review March 21, 2022 17:36
@nanderstabel nanderstabel merged commit 618045a into main Mar 21, 2022
@nanderstabel nanderstabel deleted the list-support branch March 21, 2022 21:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Introduce ListArray Introduce LIST data type
2 participants