-
Notifications
You must be signed in to change notification settings - Fork 234
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
[Rust] Minimal dataset append support #482
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious if it makes sense to make a DatasetWriter yet?
@@ -175,7 +175,7 @@ mod tests { | |||
let mut params = WriteParams::default(); | |||
params.max_rows_per_group = 10; | |||
let mut reader: Box<dyn RecordBatchReader> = Box::new(batches); | |||
Dataset::create(&mut reader, path, Some(params)) | |||
Dataset::write(&mut reader, path, Some(params)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think pyo3 will break on the name change. Don't have GHA yet for this so it's annoying. I will try to add it tonight
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that's nice. Thanks.
} | ||
} | ||
|
||
let mut fragment_id = latest_manifest.as_ref().map_or(0, |m| { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would it make sense to put max fragment in manifest?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting about how to enforce it. i.e., allow a background thread to start compact but allow writing, or just hard stop users to append new fragments.
This could be a question we need to answer when the scale is up.
Is this the same concept as |
Bring back Append support in schema evolution