feat: Support multiple external table locations#22695
Open
kumarUjjawal wants to merge 2 commits into
Open
Conversation
github-actions
Bot
added
documentation
|
Thank you for opening this pull request! Reviewer note: cargo-semver-checks reported the current version number is not SemVer-compatible with the changes in this pull request (compared against the base branch). Details |
martin-g
reviewed
Jun 1, 2026
Contributor
Author
|
Thank you @martin-g for great feedback. I have addressed the concerns. |
kumarUjjawal
force-pushed
the
feature/multi-location-external-table
branch
from
June 18, 2026 14:13
24e71e1 to
1544a99
Compare
kumarUjjawal
force-pushed
the
feature/multi-location-external-table
branch
from
June 22, 2026 11:59
1544a99 to
afc19ce
Compare
Contributor
Author
|
@Jefffrey do you have time to review this pr? |
Jefffrey
reviewed
Jul 16, 2026
Jefffrey
left a comment
Jefffrey
left a comment
Contributor
There was a problem hiding this comment.
sorry took a while for me to get around to reviewing this
| statement ok | ||
| CREATE EXTERNAL TABLE multi_loc (c1 int, c2 bigint, c3 boolean) | ||
| STORED AS CSV | ||
| LOCATION ('../core/tests/data/partitioned_csv/partition-0.csv', '../core/tests/data/partitioned_csv/partition-1.csv') |
Contributor
There was a problem hiding this comment.
does anything funky happen if you specify the same location twice?
Contributor
Author
There was a problem hiding this comment.
I will add a test
| columns, | ||
| file_type, | ||
| location, | ||
| location: _, |
Contributor
There was a problem hiding this comment.
should we add a comment here explaining why we ignore location?
|
|
||
| Ok(LogicalPlan::Ddl(DdlStatement::CreateExternalTable( | ||
| Box::new( | ||
| PlanCreateExternalTable::builder(name, location, file_type, df_schema) |
Contributor
There was a problem hiding this comment.
similarly here do we need to explain why we have to provide the first location via builder but then override (i assume?) using with_locations() after?
| name.clone(), | ||
| )), | ||
| location: location.clone(), | ||
| location: locations.first().cloned().unwrap_or_default(), |
Contributor
There was a problem hiding this comment.
is it better to just leave location empty? otherwise it could have subtle behaviour like dropping other locations if multiple were specified 🤔
| /// File type (Parquet, NDJSON, CSV, etc) | ||
| pub file_type: String, | ||
| /// Path to file | ||
| /// First path to file, retained for backwards compatibility. |
Contributor
There was a problem hiding this comment.
i do wonder if we should take this opportunity to do a clean cut to locations since its already an API change or too many downstream users rely on location and prevents us from doing so 🤔
| } | ||
| } | ||
|
|
||
| fn schemas_have_same_fields(left: &SchemaRef, right: &SchemaRef) -> bool { |
Contributor
There was a problem hiding this comment.
would a followup be exploring if we should allow union by name instead of only union by order
Contributor
Author
There was a problem hiding this comment.
This would be a good followup. Thanks!
| match &inferred_schema { | ||
| None => inferred_schema = Some((location, schema)), | ||
| Some((existing_location, existing)) | ||
| if !schemas_have_same_fields(existing, &schema) => |
Contributor
There was a problem hiding this comment.
i suppose a followup issue could be allowing users to customize this, for example filling nulls for missing columns; but this is a good initial rule
kumarUjjawal
force-pushed
the
feature/multi-location-external-table
branch
from
July 17, 2026 05:12
afc19ce to
8d22b92
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #22695 +/- ##
==========================================
- Coverage 80.66% 80.65% -0.01%
==========================================
Files 1086 1086
Lines 366694 366747 +53
Branches 366694 366747 +53
==========================================
+ Hits 295806 295818 +12
- Misses 53265 53293 +28
- Partials 17623 17636 +13 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
4 participants
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.
Which issue does this PR close?
Part of #16303.
Rationale for this change
CREATE EXTERNAL TABLEcan reference only one location today. This adds support for listing multiple explicit locations and reading them as one table.What changes are included in this PR?
LOCATION ('a.parquet', 'b.parquet')syntax.LOCATION 'a,b.parquet'as a single path, so literal commas still work.Are these changes tested?
Yes
Are there any user-facing changes?
Yes.
CREATE EXTERNAL TABLEnow accepts a parenthesized list of locations.There is also a public API change:
CreateExternalTable.locationis replaced byCreateExternalTable.locations.