Skip to content
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

feat: expand ListingSchemaProvider to support register and deregister table #3150

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

Nordalf
Copy link
Contributor

@Nordalf Nordalf commented Jan 21, 2025

Description

The ListingSchemaProvider was missing the implementations of the register and deregister traits from the SchemaProvider. These are now implemented, and to meet requirements from remote object stores, the DeltaTableBuilder is now used to pass along the storage options.

The tables property has been modified to instead of having DashMap<String, String>, it is now using Mutex<DashMap<String, Arc<dyn TableProvider>>> to support downcast_ref and mutability elsewhere.

Inspiration from: Datafusion ListingSchemaProvider

Related Issue(s)

There is no related issues. I have a clone of this with extra modifications in my own project. I find it useful to have when adding a new schema and wanted to share this with others 👍

Documentation

None

@github-actions github-actions bot added the binding/rust Issues for the Rust crate label Jan 21, 2025
@Nordalf
Copy link
Contributor Author

Nordalf commented Jan 21, 2025

Open for discussion on this one. Have had this locally for a long time and wanted to share the branch. I use this quite a lot in my project and others may find it useful to their project.

@ion-elgreco
Copy link
Collaborator

@hntd187 can you take a look? :)

Copy link
Collaborator

@hntd187 hntd187 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm supportive of this change, but I would like some changes to the internals.

@Nordalf
Copy link
Contributor Author

Nordalf commented Jan 21, 2025

Thanks for the great comments @roeap and @hntd187. I will take a look and handle them tomorrow 🚀

@Nordalf
Copy link
Contributor Author

Nordalf commented Feb 17, 2025

Thanks for the great comments @roeap and @hntd187. I will take a look and handle them tomorrow 🚀

This did not age well but hopefully I get some time this week to check out. I did figure out that DashMap already handles locking.

…n register, and added a load_table function

Signed-off-by: Alexander Falk <alexfalk7@gmail.com>
…able

Signed-off-by: Alexander Falk <alexfalk7@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binding/rust Issues for the Rust crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants