fix: fix several situations where we were incorrectly inferring the storage version #2756
Conversation
| if has_deprecated_v2_feature_flag(p.writer_feature_flags) { | ||
| DataStorageFormat::new(LanceFileVersion::Legacy) | ||
| } else { | ||
| DataStorageFormat::new(LanceFileVersion::Stable) | ||
| } |
There was a problem hiding this comment.
Good point. Let me make some migration tests for these cases
There was a problem hiding this comment.
Ok. I added some regression test cases (confirmed they detected this) and switched the logic (now tests pass)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #2756 +/- ##
==========================================
- Coverage 79.20% 79.19% -0.01%
==========================================
Files 227 227
Lines 67573 67635 +62
Branches 67573 67635 +62
==========================================
+ Hits 53521 53564 +43
- Misses 10954 10970 +16
- Partials 3098 3101 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Can we move these to the test_data folder, like the other datasets? https://github.com/lancedb/lance/tree/main/test_data
Co-authored-by: Will Jones <willjones127@gmail.com>
This could lead to corrupt datasets with a mix of v1 and v2 files.