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

Audit action field optionality #380

Merged
merged 1 commit into from
Sep 6, 2021

Conversation

fvaleye
Copy link
Collaborator

@fvaleye fvaleye commented Aug 12, 2021

@fvaleye fvaleye force-pushed the audit-actions-from-delta branch 4 times, most recently from ecc415d to 15030a6 Compare August 12, 2021 09:05
@fvaleye fvaleye marked this pull request as draft August 12, 2021 09:08
@fvaleye fvaleye force-pushed the audit-actions-from-delta branch 3 times, most recently from 2f543cd to a4e9a39 Compare August 24, 2021 10:56
@fvaleye fvaleye requested a review from houqp August 24, 2021 10:57
@fvaleye fvaleye marked this pull request as ready for review August 24, 2021 10:57
@fvaleye fvaleye added the binding/rust Issues for the Rust crate label Aug 24, 2021
@fvaleye
Copy link
Collaborator Author

fvaleye commented Aug 24, 2021

@houqp based on my investigation, tell me what you think about:

  • Keeping optional fields in Actions and Metadata when there are null by default in delta (examples from Metadata: names and description).
    Option will be used to avoid serialization/deserialization errors when reading the delta transaction log.
    I encountered several issues when reading older delta table versions <1.0 (these fields were not in the transaction logs).

  • Setting created_time in Metadata as Optional

  • Settting deletion_timestamp in Remove as Optional

  • Setting the values of data_change and extended_file_metadata with their default values when reading delta table transaction log for Remove actions

@houqp
Copy link
Member

houqp commented Sep 5, 2021

Sorry for the late response @fvaleye , catch up on notifications after getting back from vacation :)

What you proposed makes total sense to me 👍

rust/src/action.rs Show resolved Hide resolved
@fvaleye
Copy link
Collaborator Author

fvaleye commented Sep 6, 2021

Sorry for the late response @fvaleye , catch up on notifications after getting back from vacation :)

What you proposed makes total sense to me 👍

@houqp Oh, no worries at all! I hope you enjoyed it 🏖️

@fvaleye fvaleye merged commit 0a05cb4 into delta-io:main Sep 6, 2021
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.

Audit action field optionality
2 participants