feat!: allow passing down existing dataset for write #3119
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3119 +/- ##
==========================================
+ Coverage 77.87% 77.92% +0.05%
==========================================
Files 240 242 +2
Lines 81630 81903 +273
Branches 81630 81903 +273
==========================================
+ Hits 63566 63825 +259
- Misses 14830 14860 +30
+ Partials 3234 3218 -16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
wjones127
changed the title
wjones127
force-pushed
the
perf/pass-down-dataset
branch
from
5bca1f8
to
ea24413
Compare
| } | ||
|
|
||
| /// Whether to use move-stable row ids. This makes the `_rowid` column stable | ||
| /// after compaction, but not updates. |
There was a problem hiding this comment.
even during re-compaction?
interesting, not updates, will update create new rows with new _rowid?
There was a problem hiding this comment.
Even during re-compaction, yeah. We don't keep them during updates because that requires invalidating the secondary indices.
You can read more in the design docs:
- Move-stable row ids
- Primary keys - when we make row ids stable after updates too
There was a problem hiding this comment.
Thanks Will! will take a look at the design docs!
| /// | ||
| /// This can be used to stage changes or to handle "secondary" datasets | ||
| /// whose lineage is tracked elsewhere. | ||
| pub fn with_detached(mut self, detached: bool) -> Self { |
There was a problem hiding this comment.
interesting, sounds like git
There was a problem hiding this comment.
anybody is using this feature or why this feature is asked?
There was a problem hiding this comment.
It's kind of like git. Though there only one branch, and everything else is detached.
Weston added this feature to support "balanced storage" for large blob columns. Basically we used this to create a separate hidden dataset that stores the blob data, and by doing this we could compact that data at a different rate than other columns.
There was a problem hiding this comment.
Interesting, may be with 2.1 we don't need to have this kind of detached dataset for blob?
There was a problem hiding this comment.
Actually it was designed with 2.1 in mind. The ideal rows per file differs so much between small and wide columns that they need to essentially be in different datasets to offer good enough OLAP and random access performance. We wrap it up quite seamlessly though, so for the most part it feels like just another column.
| /// Pass an object store registry to use. | ||
| /// | ||
| /// If an object store is passed, this registry will be ignored. | ||
| pub fn with_object_store_registry( |
There was a problem hiding this comment.
nit: is there a macro we can use to generate this builder boilerplate?
There was a problem hiding this comment.
I could maybe write a macro, but doesn't seem that worth it to me.
BREAKING CHANGE: return value in Rust of
write_fragments()has changed toResult<Transaction>.write_fragmentsandDataset::commit()#3058InsertBuilderandCommitBuilder.CommitBuilder.LanceDataset.insert()method to modify existing datasets.Table.addresetsindex_cache_sizelancedb#1655