[For Review Only] Add APIs for deletion vector writes #1269
Conversation
github-actions
bot
added
the
breaking-change
| use crate::scan::ScanBuilder; | ||
| use crate::schema::{MapType, SchemaRef, StructField, StructType}; | ||
| use crate::snapshot::Snapshot; | ||
| use roaring::RoaringTreemap; |
There was a problem hiding this comment.
Do we want to expose 3rd party types in the Public API?
There was a problem hiding this comment.
No, I don't think we should. That will be a nightmare over ffi as well.
I think we can start with taking just a vec of indexes, or something similar, and we can add more fancy APIs as needed.
| use crate::scan::ScanBuilder; | ||
| use crate::schema::{MapType, SchemaRef, StructField, StructType}; | ||
| use crate::snapshot::Snapshot; | ||
| use roaring::RoaringTreemap; |
There was a problem hiding this comment.
No, I don't think we should. That will be a nightmare over ffi as well.
I think we can start with taking just a vec of indexes, or something similar, and we can add more fancy APIs as needed.
| self | ||
| } | ||
|
|
||
| /// Enable the writing of deletion vectors. This must be set before creating a scan builder to read data |
There was a problem hiding this comment.
I don't get the comment. A txn doesn't create a scan, so why do I need to set this if I want to scan?
| // * new_deleted_rows - The collection of new deletion vectors to apply to the files. Each | ||
| // element corresponds a non-filtered row in file_metadata. | ||
| // | ||
| // * existing_deleted_rows - The collection of existing deletion vectors corresponding to the |
There was a problem hiding this comment.
why do we need this? the data in the file_metadata already has the associated dvinfo. We might have to call visit_scan_metadata in here, but we're already going to loop over each entry in file_metadata so let's just read the dvinfo there.
Perhaps you were thinking that we could save the I/O since we might have already read the dv in order to figure out which new rows to delete?
|
closing will have new APIs with implementation up in a bit. |
3 participants
🥞 Stacked PR
Use this link to review incremental changes.