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

Add high-level checking for append-only tables #1759

Closed
junjunjd opened this issue Oct 23, 2023 · 4 comments · Fixed by #1887
Closed

Add high-level checking for append-only tables #1759

junjunjd opened this issue Oct 23, 2023 · 4 comments · Fixed by #1887
Assignees
Labels
enhancement New feature or request

Comments

@junjunjd
Copy link
Contributor

Description

Add high-level checking to delete, merge and update operations to return error immediately when the Delta table is append-only.
Use Case
#1747 adds low-level checking for append-only. This issue will add a high-level checking and thus closes #352.

Related Issue(s)
#352

@junjunjd junjunjd added the enhancement New feature or request label Oct 23, 2023
@MrPowers
Copy link
Collaborator

@junjunjd - is this done? Can we close this issue?

@junjunjd
Copy link
Contributor Author

junjunjd commented Nov 2, 2023

@junjunjd - is this done? Can we close this issue?

@MrPowers The low-level checking is added to commit function. We still need to add some high-level checking so that operations can return an error immediately before calling the commit function.

@roeap
Copy link
Collaborator

roeap commented Nov 8, 2023

#1807 adds some checks for protocol compatibility before we start the heavy lifting. To check append only, we theoretically need to see the generated actions on a table. We could try and guess what might get generated - e.g. in a merge we suspect updates - but might end up rejecting operations that may have worked. then there are more obvious ones - like delete - that we could prohibit.

Do you have some specific thoughts on where we could introduce some pre-write checks?

@junjunjd
Copy link
Contributor Author

junjunjd commented Nov 20, 2023

@roeap The ProtocolChecker looks great. I added a check_append_only method to be used for high-level checking in delete, update and write with Overwrite save mode.

You are right that for a merge, we won't be able to know if there is data change until the actions are generated. Any high-level checking would need to be added to the execute method when a Remove with data change is generated.

While going over the operations, I also noticed that filesystem check will always be rejected if the table is append-only and files_to_remove is not empty. I think it makes sense to make this operation bypass the low-level checking, otherwise append-only tables will not be able to perform any filesystem check (#1888). What do you think?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants