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

Set nulls correctly for all type of arrays/vectors #344

Merged
merged 8 commits into from
Aug 30, 2024

Conversation

y-f-u
Copy link
Contributor

@y-f-u y-f-u commented Jun 21, 2024

This sets the correct nulls for strings, struct, list, array types when copying data from arrow array to duckdb vectors.

When inserting nulls to columns with these types, the null will be correctly set instead of the default value like: "", {k: ""}, etc.

* set nulls for all possible array to vectors

* add more set nulls

* wip

* only change flat vector

* Revert "only change flat vector"

This reverts commit 90c9d75.

* add list vector nulls
@Mytherin
Copy link
Contributor

Thanks for the PR! LGTM - could you just merge with main so the CI can rerun?

@Mytherin
Copy link
Contributor

Thanks - looks like the CI now fails on the newly added test. Could you have a look when you have time?

@y-f-u
Copy link
Contributor Author

y-f-u commented Aug 8, 2024

yeah, will do. Sorry got carried away by something else, looks like an easy fix.

@y-f-u
Copy link
Contributor Author

y-f-u commented Aug 13, 2024

that's still something wrong with invalid cfg feature for time. Which is not introduced by this branch, I took a shot on fixing that but not really sure how the time features come from and how it got used.

@Mytherin
Copy link
Contributor

Thanks for looking into it - could you merge with main again? The CI should be fixed again in #375

@Mytherin Mytherin merged commit 02a0f3e into duckdb:main Aug 30, 2024
4 checks passed
@Mytherin
Copy link
Contributor

Thanks!

@phillipleblanc phillipleblanc deleted the fix-more-null-issues-in-vectors branch September 2, 2024 01:22
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.

4 participants