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: implement clone for DeltaTable struct #2160

Merged
merged 3 commits into from
Feb 1, 2024

Conversation

mightyshazam
Copy link
Contributor

Description

This PR adds the derive(Clone) attribute to theDeltaTable struct.

Related Issue(s)

Documentation

@github-actions github-actions bot added the binding/rust Issues for the Rust crate label Feb 1, 2024
@roeap
Copy link
Collaborator

roeap commented Feb 1, 2024

@mightyshazam - thanks, I actually have been waiting for this PR 😆.

For a looong time folks wanted DeltaTable to be Cloneable and we have always rejected it since cloning the state used to be potentially ridiculously expensive ... now its practically for free 🥲.

So I am fine to merge this. However, I'd be curious about the use case. Specifically in most situations I would recommend cloning the state itself, as DeltaTable just exposes some higher level functionality on top of it and may not have a valid state yet, which forces you to deal with a lot of Options and/or Results which you avoid by just using the Snapshot.

@mightyshazam
Copy link
Contributor Author

@mightyshazam - thanks, I actually have been waiting for this PR 😆.

For a looong time folks wanted DeltaTable to be Cloneable and we have always rejected it since cloning the state used to be potentially ridiculously expensive ... now its practically for free 🥲.

So I am fine to merge this. However, I'd be curious about the use case. Specifically in most situations I would recommend cloning the state itself, as DeltaTable just exposes some higher level functionality on top of it and may not have a valid state yet, which forces you to deal with a lot of Options and/or Results which you avoid by just using the Snapshot.

I'm working on dotnet bindings for delta rs, and I wanted to enable SQL querying. That requires passing the struct to the register_table method of the data fusion context. Since I have the table stored in a raw pointer, I can't pass ownership to the context. Instead, I would have to construct a new table.
I saw that cloning is easy and cheap, but it wasn't implemented.

@rtyler rtyler added this to the Rust v0.17 milestone Feb 1, 2024
@rtyler rtyler enabled auto-merge (rebase) February 1, 2024 21:07
@rtyler rtyler merged commit 18232bd into delta-io:main Feb 1, 2024
20 checks passed
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.

3 participants