Repository navigation
test(datafusion): cover the factory's one-location check; fix fork branch notes - #54
Merged
krinart merged 1 commit intoOct 3, 2026
Conversation
…anch notes The DataFusion 55 port rejects CREATE EXTERNAL TABLE commands that carry zero or several locations, but only the one-location path was tested. Add a test for each case. The arrow-rs and DataFusion pins are the heads of spiceai-59 and spiceai-55, not of the -patches branches the comments named.
There was a problem hiding this comment.
Copilot review overview
🟢 Approved
The focused tests accurately cover both invalid location counts, and the comment-only branch corrections match the stated revisions.
Review effort: Balanced
Findings: None
What changed in this PR
Adds regression coverage for DataFusion external-table location validation and corrects fork branch annotations.
Changes:
- Tests rejection of zero and multiple metadata locations.
- Corrects Arrow and DataFusion fork branch comments.
| File | Description |
|---|---|
crates/integrations/datafusion/src/table/table_provider_factory.rs |
Adds location-count validation tests. |
Cargo.toml |
Corrects pinned revision branch notes. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
lukekim
added a commit
that referenced
this pull request
Oct 3, 2026
…anch notes (#54) The DataFusion 55 port rejects CREATE EXTERNAL TABLE commands that carry zero or several locations, but only the one-location path was tested. Add a test for each case. The arrow-rs and DataFusion pins are the heads of spiceai-59 and spiceai-55, not of the -patches branches the comments named.
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
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.
Review fixes for #50. That PR's head branch,
spiceai-0.10.1-df-55-patches, only takes changes through a PR, so they come here.IcebergTableProviderFactory::createreject a command with zero or several locations (An iceberg external table takes exactly one metadata file location, got N). Only the one-location success path was tested. This addstest_create_rejects_zero_locationsandtest_create_rejects_multiple_locations(Copilot on #50).Cargo.toml. The[patch.crates-io]notes namedspiceai-59-patchesandspiceai-55-patches, but the pinned revisions are the heads of spiceai/arrow-rsspiceai-59(2e2cc330) and spiceai/datafusionspiceai-55(02550cf9). Neither revision is on the-patchesbranch. Comment-only change (Copilot on #50).Test plan
cargo test -p iceberg-datafusion --lib table_provider_factory: on a0bd6b02 passed, with this change4 passed; 0 failed.cmd.locations.first(), the run gives2 passed; 2 failed. Zero locations reachesFailed to read file : No such file or directory (os error 2), and two locations silently opens the first.cargo fmt --all -- --check,taplo fmt --checkandcargo clippy -p iceberg-datafusion --all-targets --all-features -- -D warningsall exit 0.spiceai-59...2e2cc330isidentical, whilespiceai-59-patches...2e2cc330isdiverged(6 ahead, 3 behind).spiceai-55...02550cf9isidentical, whilespiceai-55-patches...02550cf9isdiverged(13 ahead, 27 behind).