Adding integrations tests + Fixes #10
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
paleolimbot
approved these changes
Mar 17, 2025
There was a problem hiding this comment.
This would benefit from a formatter (I'll circle back and add ruff to pre-commit)
|
Thanks for the quick look @paleolimbot I'll leave the Python test refactoring to you then! Will merge this later if CI passes :-). |
Mytherin
added a commit
to duckdb/duckdb
that referenced
this pull request
Mar 21, 2025
…write Null columns to arrow (#16711) This PR fixes the issues described in the title and encountered in: paleolimbot/duckdb-nanoarrow#10 Fix: duckdblabs/duckdb-internal#4451 I've tagged the merge to `v1.2` for simplicity, but this can also be retargeted to `main`, I don't have a strong preference.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As suggested in this issue, I've added tests using the same list of files that Nanoarrow currently tests with.
Some files are not yet supported, but the reasons are related to DuckDB's core Arrow integration rather than this extension.
The tests cover both the file reader/writer and the buffer reader/writer, validating all results by reading the files directly with PyArrow.
Files Not Currently Tested
generated_decimal256.stream: This type is not supported in DuckDB's core.generated_interval.stream: DuckDB stores time as us, which causes a multiplication overflow at this precision, leading to errors.generated_union.stream: This type is not supported in DuckDB's core.Specifically for the Buffer Writer
generated_map_non_canonical.streamandgenerated_map.stream: The schema of the key is not set to NOT NULL in DuckDB's core, which later causes a validation failure.generated_null.stream: A cast is missing in the core integration.Otherwise, this PR mainly includes fixes to ensure all files pass correctly.
I think this was the last step before submitting this extension to the community, what do you think @paleolimbot?
Then for v1.3, I'll adjust the buffer call as described in #9