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

Don't ensure_table_uri when creating a table with_log_store #2036

Closed
gruuya opened this issue Jan 5, 2024 · 3 comments
Closed

Don't ensure_table_uri when creating a table with_log_store #2036

gruuya opened this issue Jan 5, 2024 · 3 comments
Labels
binding/rust Issues for the Rust crate enhancement New feature or request

Comments

@gruuya
Copy link
Contributor

gruuya commented Jan 5, 2024

Description

Currently when a CreateBuilder is provided with a log store, it will try to create a URL from it's root (via ensure_table_uri), and then re-convert that to a String

let (storage_url, table) = if let Some(log_store) = self.log_store {
(
ensure_table_uri(log_store.root_uri())?.as_str().to_string(),
DeltaTable::new(log_store, Default::default()),
)

Besides potentially create missing dirs in the case of local FS, I don't think this round trip is actually needed in other cases (e.g. cloud URLs).

The issue this leads to for us is that ensure_table_uri in turn calls resolve_uri_type, which errors out if the URL scheme is not for one of the registered object store factories.

Use Case

I'd like to be able to use with_log_store on the CreateBuilder without having to register an object store factory beforehand.

In other words I'd like to be able to re-use the object/log store that I keep an Arc to without re-creating one from scratch every time.

Related Issue(s)

@gruuya gruuya added the enhancement New feature or request label Jan 5, 2024
@gruuya
Copy link
Contributor Author

gruuya commented Jan 5, 2024

I guess one resolution to this would be to have my ObjectStoreFactory just return the pre-configured ones according to the URL, which is also fine.

@rtyler
Copy link
Member

rtyler commented Jan 5, 2024

@gruuya I see you're following main closely 😆 Are you working on a different storage integration, or just concerned with the goofy file creation behavior in core here?

@rtyler rtyler added the binding/rust Issues for the Rust crate label Jan 5, 2024
@gruuya
Copy link
Contributor Author

gruuya commented Jan 5, 2024

I see you're following main closely

Yeah, pretty nice to see the pace of development here, kudos :)

Are you working on a different storage integration, or just concerned with the goofy file creation behavior in core here?

I was mostly confused by how previously I had an object/log store instance that I'd supply to the CreateBuilder and be done with it, whereas now I need to register the factory as well.

Come to think of it, this mechanism not only makes sense broadly speaking but it's a pretty good idea actually, given that I've been contemplating how to introduce dynamic object/log store creation down the road.

Sorry for the noise and thanks!

@gruuya gruuya closed this as completed Jan 5, 2024
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 enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants