-
Notifications
You must be signed in to change notification settings - Fork 8
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
Support selecting from local files #152
base: main
Are you sure you want to change the base?
Conversation
@@ -15,18 +15,18 @@ | |||
// specific language governing permissions and limitations |
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.
I renamed the tests from 'extensions' to 'execution' to better reflect it is testing execution in general rather than only extension execution
// session_ctx.enable_url_table(); | ||
// expecting that to work, but it didn't as the code returns a new session context | ||
// instead of modifying the existing one | ||
let session_ctx = session_ctx.enable_url_table(); |
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.
this is the only change needed (it is a nice API thanks to @goldmedal)
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.
this is pretty awesome
This works pretty great -- but it is blocked on the delta.rs upgrade to datafusion 4.2 |
// TODO file a ticket to make the API consuming!! | ||
// I did like | ||
// session_ctx.enable_url_table(); | ||
// expecting that to work, but it didn't as the code returns a new session context | ||
// instead of modifying the existing one |
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.
i agree that would be nicer
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.
apache/datafusion#12551 tracking tickeet
Draft:
Closes #125
Hooks up the DynamicCatalogFileProvoder and tests with it