feat: once a table has been created with v1 or v2 format then it should always use that format #2435

westonpace · 2024-06-04T12:53:37Z

We want to avoid mixed-format datasets. In theory, they should work. However, it just expands our list of potential test scenarios without any real benefit. Also, its easier to expose the use_legacy_format flag in lancedb if we only have to modify the create table operation.

…ays use that format

westonpace · 2024-06-04T12:56:06Z

rust/lance/src/dataset.rs

+                let use_v2 = matches!(
+                    d.schema()
+                        .metadata
+                        .get("lance:use_lance_v2")
+                        .map(|s| s.as_str()),
+                    Some("true")
+                );
+                params.use_legacy_format = !use_v2;
+            }
+        } else {
+            if !params.use_legacy_format {
+                schema
+                    .metadata
+                    .insert("lance:use_lance_v2".to_string(), "true".to_string());


@wjones127 I'm using the schema metadata to store a sort of "soft feature flag". This is potentially dangerous (users can change the dataset schema and erase this) but since v2 support is only experimental this seems ok.

On the other hand we could use a feature flag but this flag isn't going to be permanent (once v2 is stable we will no longer need it) and so I wasn't sure if we wanted to waste a bit space in our feature flags (though I suppose we could "return it to the pool" once we're done with v2).

Let me know if you would prefer a feature flag.

On second thought, I've changed the PR to use feature flags. It makes sense. If anyone were writing an independent lance table reader then this would be a significant feature change that would make the resulting tables incompatible.

wjones127 · 2024-06-04T17:27:37Z

rust/lance-table/src/feature_flags.rs

+/// The first bit that is invalid as a feature flag
+pub const FLAG_INVALID: u64 = 8;


Technically, not really invalid, but rather unknown.

Changed to FLAG_UNKNOWN.

wjones127 · 2024-06-04T17:28:05Z

rust/lance-table/src/feature_flags.rs

@@ -55,11 +60,15 @@ pub fn apply_feature_flags(manifest: &mut Manifest) -> Result<()> {
 }

 pub fn can_read_dataset(reader_flags: u64) -> bool {
-    reader_flags <= 3
+    reader_flags <= 7


Should this be

Suggested change

reader_flags <= 7

reader_flags < FLAG_INVALID

wjones127 · 2024-06-04T17:32:39Z

rust/lance/src/dataset.rs

@@ -1633,6 +1653,8 @@ mod tests {
    #[rstest]
    #[tokio::test]
    async fn test_write_manifest(#[values(false, true)] use_legacy_format: bool) {
+        use lance_table::feature_flags::FLAG_INVALID;


Seems like you should change the assert_eq!(manifest.writer_feature_flags, 0); below to have the V2 flag if expected.

Good catch. Fixed.

[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com) This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [lance](https://togithub.com/lancedb/lance) | dependencies | minor | `0.10.16` -> `0.12.0` | --- ### Release Notes <details> <summary>lancedb/lance (lance)</summary> ### [`v0.12.1`](https://togithub.com/lancedb/lance/releases/tag/v0.12.1) [Compare Source](https://togithub.com/lancedb/lance/compare/v0.12.0...v0.12.1)  #### What's Changed ##### Bug Fixes 🐛 - fix: incorrect chunking was making lance datasets use too much RAM by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2438 **Full Changelog**: lancedb/lance@v0.12.0...v0.12.1 ### [`v0.12.0`](https://togithub.com/lancedb/lance/releases/tag/v0.12.0) [Compare Source](https://togithub.com/lancedb/lance/compare/v0.11.1...v0.12.0)  #### What's Changed ##### Breaking Changes 🛠 - feat: change dataset uri to return full qualified url instead of object store path by [@eddyxu](https://togithub.com/eddyxu) in [lancedb/lance#2416 ##### New Features 🎉 - feat: new shuffler by [@BubbleCal](https://togithub.com/BubbleCal) in [lancedb/lance#2404 - feat: new index builder by [@BubbleCal](https://togithub.com/BubbleCal) in [lancedb/lance#2401 - feat: stable row id manifest changes by [@wjones127](https://togithub.com/wjones127) in [lancedb/lance#2363 - feat: once a table has been created with v1 or v2 format then it should always use that format by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2435 ##### Bug Fixes 🐛 - fix: fix file writer which was not writing page buffers in the correct order by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2413 ##### Other Changes - refactor: refactor logical decoders into "field decoders" by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2407 - refactor: rename use_experimental_writer to use_legacy_format by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2433 - refactor: minor refactor to allow I/O scheduler to be cloned in page schedulers by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2432 **Full Changelog**: lancedb/lance@v0.11.1...v0.12.0 ### [`v0.11.1`](https://togithub.com/lancedb/lance/releases/tag/v0.11.1) [Compare Source](https://togithub.com/lancedb/lance/compare/v0.11.0...v0.11.1)  #### What's Changed ##### New Features 🎉 - feat(java): support jdk8 by [@LuQQiu](https://togithub.com/LuQQiu) in [lancedb/lance#2362 - feat: support kmode with hamming distance by [@eddyxu](https://togithub.com/eddyxu) in [lancedb/lance#2366 - feat: row id index structures (experimental) by [@wjones127](https://togithub.com/wjones127) in [lancedb/lance#2303 - feat: update merge_insert to add statistics for inserted, updated, deleted rows by [@raunaks13](https://togithub.com/raunaks13) in [lancedb/lance#2357 - feat: define Flat index as a scan over VectorStorage by [@chebbyChefNEQ](https://togithub.com/chebbyChefNEQ) in [lancedb/lance#2380 - feat: add some schema utility methods to the v2 reader/writer by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2389 - feat: general compression for value page buffer by [@niyue](https://togithub.com/niyue) in [lancedb/lance#2368 - feat: make the index cache size (in bytes) available by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2381 - feat: add special uri scheme to use CloudFileReader for local fs by [@chebbyChefNEQ](https://togithub.com/chebbyChefNEQ) in [lancedb/lance#2402 - feat: add encoder utilities for pushdown by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2388 ##### Bug Fixes 🐛 - fix: concat batches before writing to avoid small IO slow down by [@chebbyChefNEQ](https://togithub.com/chebbyChefNEQ) in [lancedb/lance#2384 - fix: low recall if the num partitions is more than num rows by [@BubbleCal](https://togithub.com/BubbleCal) in [lancedb/lance#2386 - fix: f32 reduce_min for x86 by [@heiher](https://togithub.com/heiher) in [lancedb/lance#2385 - fix: fix incorrect validation logic in updater by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2408 ##### Performance Improvements 🚀 - perf: make VectorStorage and DistCalculator static to generate better code by [@BubbleCal](https://togithub.com/BubbleCal) in [lancedb/lance#2355 - perf: optimize IO path for reading manifest by [@wjones127](https://togithub.com/wjones127) in [lancedb/lance#2396 ##### Other Changes - refactor: make proto conversion fallible and not copy by [@wjones127](https://togithub.com/wjones127) in [lancedb/lance#2371 - refactor: separate take and schema evolution impls to own files by [@wjones127](https://togithub.com/wjones127) in [lancedb/lance#2372 - Revert "fix: concat batches before writing to avoid small IO slow down ([#2384](https://togithub.com/lancedb/lance/issues/2384))" by [@chebbyChefNEQ](https://togithub.com/chebbyChefNEQ) in [lancedb/lance#2387 - refactor: shuffle around v2 metadata sections to allow read-on-demand statistics by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2400 #### New Contributors - [@niyue](https://togithub.com/niyue) made their first contribution in [lancedb/lance#2368 - [@heiher](https://togithub.com/heiher) made their first contribution in [lancedb/lance#2385 **Full Changelog**: lancedb/lance@v0.11.0...v0.11.1 ### [`v0.11.0`](https://togithub.com/lancedb/lance/releases/tag/v0.11.0) [Compare Source](https://togithub.com/lancedb/lance/compare/v0.10.18...v0.11.0)  #### What's Changed ##### Breaking Changes 🛠 - feat(rust)!: use BoxedError in Error::IO by [@broccoliSpicy](https://togithub.com/broccoliSpicy) in [lancedb/lance#2329 ##### New Features 🎉 - feat: add v2 support to fragment merge / update paths by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2311 - feat: add priority to I/O scheduler by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2315 - feat: add take_rows operation to the v2 file reader's python bindings by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2331 - feat: added example for reading and writing dataset in rust by [@raunaks13](https://togithub.com/raunaks13) in [lancedb/lance#2349 - feat: new HNSW implementation by [@BubbleCal](https://togithub.com/BubbleCal) in [lancedb/lance#2353 - feat: add fragment take / fixed-size-binary support to v2 format by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2354 ##### Bug Fixes 🐛 - fix: recognize a simple expression like 'is_foo' as a scalar index query by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2356 - fix: rework list encoder to handle list-struct by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2344 - fix: minor bug fixes for v2 by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2361 ##### Documentation 📚 - docs: clearify comments in table.proto -> message DataFragment -> physical_rows by [@broccoliSpicy](https://togithub.com/broccoliSpicy) in [lancedb/lance#2346 ##### Performance Improvements 🚀 - perf: use the file metadata cache in scalar indices by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2330 ##### Other Changes - chore: remove `m_max` and `use_heuristic` params from HNSW builder by [@BubbleCal](https://togithub.com/BubbleCal) in [lancedb/lance#2336 - fix(java): fix JNI jar loader issue by [@LuQQiu](https://togithub.com/LuQQiu) in [lancedb/lance#2340 - ci: fix labeler permissions by [@wjones127](https://togithub.com/wjones127) in [lancedb/lance#2348 - fix: rework decoding to fix bugs in nested struct decoding by [@westonpace](https://togithub.com/westonpace) in [lancedb/lance#2337 #### New Contributors - [@broccoliSpicy](https://togithub.com/broccoliSpicy) made their first contribution in [lancedb/lance#2346 - [@raunaks13](https://togithub.com/raunaks13) made their first contribution in [lancedb/lance#2349 **Full Changelog**: lancedb/lance@v0.10.18...v0.11.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://developer.mend.io/github/spiraldb/vortex).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

Once a table has been created with v1 or v2 format then it should alw…

6a34203

…ays use that format

github-actions bot added enhancement New feature or request python labels Jun 4, 2024

westonpace commented Jun 4, 2024

View reviewed changes

westonpace added 3 commits June 4, 2024 07:34

Apply clippy suggestion

cc83810

Move to using feature flags instead of schema

a05f988

Fix feature flag impl

3f465e3

wjones127 reviewed Jun 4, 2024

View reviewed changes

Apply PR sugestions

bb1e311

westonpace requested a review from wjones127 June 4, 2024 19:35

wjones127 approved these changes Jun 4, 2024

View reviewed changes

westonpace closed this Jun 4, 2024

westonpace reopened this Jun 4, 2024

westonpace merged commit 9c9ac51 into lancedb:main Jun 4, 2024
27 of 29 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: once a table has been created with v1 or v2 format then it should always use that format #2435

feat: once a table has been created with v1 or v2 format then it should always use that format #2435

westonpace commented Jun 4, 2024

westonpace Jun 4, 2024

westonpace Jun 4, 2024

wjones127 Jun 4, 2024

westonpace Jun 4, 2024

wjones127 Jun 4, 2024

westonpace Jun 4, 2024

wjones127 Jun 4, 2024

westonpace Jun 4, 2024

		/// The first bit that is invalid as a feature flag
		pub const FLAG_INVALID: u64 = 8;

feat: once a table has been created with v1 or v2 format then it should always use that format #2435

feat: once a table has been created with v1 or v2 format then it should always use that format #2435

Conversation

westonpace commented Jun 4, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment