feat(transaction): Implement TransactionAction for FastAppendAction#1448
feat(transaction): Implement TransactionAction for FastAppendAction#1448liurenjie1024 merged 7 commits intoapache:mainfrom
Conversation
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @CTTY for this pr, generally looks good!
…ducer, remove add_parquet_files
liurenjie1024
left a comment
There was a problem hiding this comment.
Thanks @CTTY for this pr, LGTM!
| output: OutputFile, | ||
| snapshot_id: Option<i64>, | ||
| key_metadata: Vec<u8>, | ||
| key_metadata: Option<Vec<u8>>, |
There was a problem hiding this comment.
This is an API change, would appreciate if we could call it out at PR description and release notes.
There was a problem hiding this comment.
Sorry about that! I've updated the PR description.
cc @liurenjie1024 probably related to the release process: Is there a "staging release notes" that we can start putting changes like this into as breaking changes?
There was a problem hiding this comment.
I'm thinking about adding a breaking label to pr so that it standout when we generate release notes.
| SnapshotProducer::validate_added_data_files(table, &self.added_data_files)?; | ||
|
|
||
| /// Finished building the action and apply it to the transaction. | ||
| pub async fn apply(self) -> Result<Transaction> { |
There was a problem hiding this comment.
This is an API change, would appreciate PR description and release notes, even provide a small example for no-op migration.
There was a problem hiding this comment.
Updated the PR description accordingly as well
Which issue does this PR close?
Related Issues:
What changes are included in this PR?
BREAKING CHANGES:
ManifestWriternow takeskey_metadata: Option<Vec<u8>>instead ofkey_metadata: Vec<u8>FastAppendAction::applywithcommitas an implementation ofTransactionActionGeneral:
TransactionActionforFastAppendActionSnapshotProduceActiontoSnapshotProducerSnapshotProduceActionan stateless helper class that helps with fast_append::commit (as discussed here)Are these changes tested?
Added unit tests