-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Minor: Move ObjectStoreRegistry
to datafusion_execution crate
#5478
Conversation
@@ -103,3 +103,52 @@ impl From<ObjectMeta> for PartitionedFile { | |||
} | |||
} | |||
} | |||
|
|||
#[cfg(test)] | |||
mod tests { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
these tests are of ListingTableUrl
which is in core/src/datasource/listing I moved them here
@@ -255,34 +245,4 @@ mod tests { | |||
ObjectStoreUrl::parse("s3://username:password@host:123/foo").unwrap_err(); | |||
assert_eq!(err.to_string(), "Execution error: ObjectStoreUrl must only contain scheme and authority, got: /foo"); | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved this into the datasource/listing module
@@ -26,9 +26,11 @@ use std::collections::HashMap; | |||
|
|||
use crate::datasource::datasource::TableProviderFactory; | |||
use crate::datasource::listing_table_factory::ListingTableFactory; | |||
use crate::datasource::object_store::ObjectStoreRegistry; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the point of this PR is to remove the dependency here on datasource
The other references to datasource are removed in #5477
Benchmark runs are scheduled for baseline = 07b07d5 and contender = 55c7c4d. 55c7c4d is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Part of #1754
Rationale for this change
I am trying to extract the physical_plan code into its own crate; and to do so I need to remove the circular dependencies between core --> datasource --> execution --> datasource
The
ObjectStoreRegistry
is used at runtime, so move it into the datafusion_execution crateSee more details in #1754 (comment)
What changes are included in this PR?
ObjectStoreRegistry
to datafusion_execution crateAre these changes tested?
Covered by existing tests
Are there any user-facing changes?
No, I added a
pub use
so this code movement should not affect users