Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions docs/src/format/table/transaction.md
Original file line number Diff line number Diff line change
Expand Up @@ -505,13 +505,24 @@ The following operations are retryable conflicts with DataOverlay:
overlay→base fold changes physical row addresses or consumes the overlays, so
the overlay's offsets are no longer valid; the writer must re-read the new
fragment, recompute, and retry.
- Merge (always)

DataOverlay is compatible with another DataOverlay (any fields), Append, Delete,
and DataReplacement or a column rewrite (Update with `REWRITE_COLUMNS`) of the same
field, because all of these preserve physical row addresses: overlay offsets stay
valid, the overlay is newer and wins its covered cells, and the version gate
excludes those cells from any rebuilt index.
- Merge (always).
- A row-moving Update that touches an overlaid fragment — a delete-and-reinsert
update (any update that is not a `REWRITE_COLUMNS` column rewrite) relocates the
updated rows into new fragments, so the overlay's physical offsets no longer
address them; the writer must re-read and retry.

Comment on lines +508 to +513

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Clarify that the row-moving Update conflict is row-level, not fragment-level.

As written, this reads as "any row-moving Update touching an overlaid fragment conflicts." In practice the conflict resolver only retries when the specific rows the update relocated intersect the overlay's covered offsets (checked by diffing deletion vectors at commit-finish time); a row-moving update on the same fragment whose moved rows are disjoint from the overlay's coverage succeeds without conflict.

As per coding guidelines: "Describe algorithms in full detail, including parameters, precision, ordering, normalization bounds, and implementation steps; do not reference an algorithm by name alone."

✏️ Suggested wording
-- A row-moving Update that touches an overlaid fragment — a delete-and-reinsert
-  update (any update that is not a `REWRITE_COLUMNS` column rewrite) relocates the
-  updated rows into new fragments, so the overlay's physical offsets no longer
-  address them; the writer must re-read and retry.
+- A row-moving Update (any update that is not a `REWRITE_COLUMNS` column rewrite)
+  whose relocated rows intersect the overlay's covered offsets on the same
+  fragment — the moved rows are re-created from the pre-overlay base, so the
+  overlay's values for those specific offsets are lost; conflict detection
+  compares the update's moved-row set against the overlay's coverage bitmap
+  (via the deletion-vector delta at commit time), so a row-moving update whose
+  moved rows don't overlap the overlay's coverage does not conflict.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
- Merge (always).
- A row-moving Update that touches an overlaid fragment — a delete-and-reinsert
update (any update that is not a `REWRITE_COLUMNS` column rewrite) relocates the
updated rows into new fragments, so the overlay's physical offsets no longer
address them; the writer must re-read and retry.
- A row-moving Update (any update that is not a `REWRITE_COLUMNS` column rewrite)
whose relocated rows intersect the overlay's covered offsets on the same
fragment — the moved rows are re-created from the pre-overlay base, so the
overlay's values for those specific offsets are lost; conflict detection
compares the update's moved-row set against the overlay's coverage bitmap
(via the deletion-vector delta at commit time), so a row-moving update whose
moved rows don't overlap the overlay's coverage does not conflict.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@docs/src/format/table/transaction.md` around lines 508 - 513, Update the
Merge documentation to state that row-moving Update conflicts are determined at
the row level, not merely by sharing a fragment: conflict occurs only when the
update’s relocated rows intersect the overlay’s covered offsets, as determined
by diffing deletion vectors at commit-finish time; disjoint relocated rows on
the same fragment must succeed. Describe this retry condition and ordering
explicitly in the relevant bullet.

Source: Coding guidelines

DataOverlay is compatible with another DataOverlay (any fields), Append, Delete, a
`REWRITE_COLUMNS` column rewrite, and DataReplacement, because all of these
preserve physical row addresses: overlay offsets stay valid, the overlay is newer
and wins its covered cells, and the version gate excludes those cells from any
rebuilt index.

When a DataReplacement or a `REWRITE_COLUMNS` update writes new base values for a
field, it supersedes any older overlay on that field: the writer tombstones the
overlay's entry for the rewritten field — replacing the field id with the obsolete
sentinel, as with obsolete base columns — so the fresh base values are not silently
shadowed. Overlay entries for other fields are preserved, and an overlay left with
no live fields is dropped.

### UpdateMemWalState

Expand Down
3 changes: 3 additions & 0 deletions java/lance-jni/src/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -828,6 +828,9 @@ impl FromJObjectWithEnv<Fragment> for JObject<'_> {
row_id_meta,
created_at_version_meta,
last_updated_at_version_meta,
// Overlays are not exposed to Java yet, and the reverse conversion
// does not export them, so this round-trip is overlay-free.
overlays: vec![],
})
}
}
Expand Down
3 changes: 3 additions & 0 deletions python/src/fragment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -825,6 +825,9 @@ impl FromPyObject<'_, '_> for PyLance<Fragment> {
row_id_meta,
last_updated_at_version_meta,
created_at_version_meta,
// Overlays are not exposed to Python yet, and the reverse conversion
// does not export them, so this round-trip is overlay-free.
overlays: vec![],
Comment on lines +828 to +830

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -eu

rg -n -C5 \
  'PyLance<Fragment>|FromPyObject.*Fragment|extract.*Fragment|Fragment.*(write|commit|append|update)' \
  python rust/lance

Repository: lance-format/lance

Length of output: 27436


🏁 Script executed:

#!/bin/bash
set -eu

sed -n '800,910p' python/src/fragment.rs
printf '\n---\n'
rg -n -C3 'struct Fragment|overlays' rust/lance/src python/src | head -n 80

Repository: lance-format/lance

Length of output: 9915


🏁 Script executed:

#!/bin/bash
set -eu

# Inspect the Rust Fragment definition and Python conversion code paths.
rg -n -C4 'pub struct Fragment|overlays:' rust/lance/src python/src
printf '\n=== fragment.rs around conversions ===\n'
sed -n '800,910p' python/src/fragment.rs

Repository: lance-format/lance

Length of output: 50374


🏁 Script executed:

#!/bin/bash
set -eu

# Find where Fragment metadata is converted back to Python and whether overlays are included.
sed -n '876,930p' python/src/fragment.rs
printf '\n---\n'
sed -n '1,140p' rust/lance/src/dataset/fragment.rs
printf '\n---\n'
rg -n -C3 'overlay|overlays|DataOverlay|OverlayCoverage' rust/lance/src/lance_table rust/lance/src | head -n 120

Repository: lance-format/lance

Length of output: 17299


🏁 Script executed:

#!/bin/bash
set -eu

# Check whether the Python package exposes overlays on Fragment at all.
rg -n -C3 'class Fragment|_Fragment|overlays|Fragment =' python/python/lance/fragment.py python/src/fragment.rs
printf '\n---\n'
sed -n '1,120p' python/python/lance/fragment.py
printf '\n---\n'
sed -n '876,930p' python/src/fragment.rs

Repository: lance-format/lance

Length of output: 9905


🌐 Web query:

Lance Python fragment overlays PyLance Fragment overlays not exposed

💡 Result:

The term "fragment overlays" is not a concept or feature within the Microsoft Pylance extension for Visual Studio Code [1][2]. Based on your query, it is highly likely that you are encountering a misunderstanding of terminology related to the LanceDB project. In the context of the Lance data format, "fragment overlays" (often referred to as "Data Overlay Files") are a technical feature used for data versioning, modifications, and storage [3]. Research indicates that the LanceDB project uses a Python interface, frequently referred to in repository code as pylance or associated with the lance Python package, which manages fragments and overlays [4][5]. If you are experiencing issues where certain features or metadata related to these overlays are not "exposed," it is likely that: 1. You are interacting with the LanceDB Python library (sometimes colloquially called "pylance" in internal development contexts) [5], not the Microsoft Pylance VS Code extension [1]. 2. The specific feature you are looking for may be internal, currently gated by feature flags (such as LANCE_ENABLE_DATA_OVERLAY_FILES), or not yet exposed in the public-facing Python API [3]. To resolve this, you should check the official LanceDB documentation or GitHub repository issues regarding Data Overlay Files [3], rather than searching for Microsoft Pylance settings. If you intended to ask about the Microsoft Pylance extension, be aware that "overlays" is not part of its vocabulary, and any issues with missing symbols are likely related to Python path configuration or namespace package resolution [6][7].

Citations:


🏁 Script executed:

#!/bin/bash
set -eu

# Inspect the public Python fragment API around FragmentMetadata and LanceFragment.
sed -n '59,140p' python/python/lance/fragment.py
printf '\n---\n'
sed -n '286,470p' python/python/lance/fragment.py
printf '\n---\n'
rg -n -C2 'overlays|FragmentMetadata|_Fragment' python/python/lance/fragment.py python/src/fragment.rs

Repository: lance-format/lance

Length of output: 21405


🏁 Script executed:

#!/bin/bash
set -eu

# Inspect Python entry points that consume fragment collections for commit/write paths.
rg -n -C4 '_write_fragments_transaction|_write_fragments|fragments:|FragmentMetadata|LanceFragment' python/python/lance/fragment.py python/python/lance/dataset.py python/python/lance/transaction.py python/src/transaction.rs

Repository: lance-format/lance

Length of output: 41740


Preserve overlays in PyLance<Fragment> conversion.
python/src/fragment.rs:828-830 drops overlays, but LanceFragment.merge/update_columns return fragment metadata that can be committed again. That silently loses overlay files; carry them through opaquely or reject non-empty overlays here.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@python/src/fragment.rs` around lines 828 - 830, The PyLance<Fragment>
conversion currently discards overlays by assigning an empty vector, causing
committed merge/update results to lose overlay files. Update the conversion
implementation and its reverse conversion to preserve overlays opaquely, or
explicitly reject fragments with non-empty overlays instead of silently dropping
them; use the relevant PyLance and Lance Fragment conversion methods around the
overlays field.

}))
}
}
Expand Down
130 changes: 127 additions & 3 deletions rust/lance-table/src/feature_flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,22 @@ pub const FLAG_TABLE_CONFIG: u64 = 8;
pub const FLAG_BASE_PATHS: u64 = 16;
/// Disable writing transaction file under _transaction/, this flag is set when we only want to write inline transaction in manifest
pub const FLAG_DISABLE_TRANSACTION_FILE: u64 = 32;
/// Fragments contain data overlay files, which supply new values for a subset of
/// cells without rewriting base data files. A reader that does not understand
/// overlays must refuse the dataset, since ignoring an overlay would silently
/// return stale base values.
///
/// Data overlay files are not yet a released feature: in release builds this flag
/// is treated as unknown (so a release reader/writer refuses an overlay dataset)
/// unless [`ENABLE_UNSTABLE_DATA_OVERLAY_FILES_ENV`] is set, which lets benchmarks opt in.
/// Debug builds always understand it so tests exercise the path.
pub const FLAG_UNSTABLE_DATA_OVERLAY_FILES: u64 = 64;
/// The first bit that is unknown as a feature flag
pub const FLAG_UNKNOWN: u64 = 64;
pub const FLAG_UNKNOWN: u64 = 128;

/// Environment variable that opts a release build into reading and writing data
/// overlay files before the feature is generally released.
pub const ENABLE_UNSTABLE_DATA_OVERLAY_FILES_ENV: &str = "LANCE_ENABLE_UNSTABLE_DATA_OVERLAY_FILES";
Comment thread
wjones127 marked this conversation as resolved.

/// Set the reader and writer feature flags in the manifest based on the contents of the manifest.
pub fn apply_feature_flags(
Expand Down Expand Up @@ -71,18 +85,62 @@ pub fn apply_feature_flags(
manifest.writer_feature_flags |= FLAG_BASE_PATHS;
}

// Overlay files change cell values on read, so a reader that ignores them
// would return stale base values. Both readers and writers must understand
// them.
let has_overlays = manifest
.fragments
.iter()
.any(|frag| !frag.overlays.is_empty());
if has_overlays {
manifest.reader_feature_flags |= FLAG_UNSTABLE_DATA_OVERLAY_FILES;
manifest.writer_feature_flags |= FLAG_UNSTABLE_DATA_OVERLAY_FILES;
}

if disable_transaction_file {
manifest.writer_feature_flags |= FLAG_DISABLE_TRANSACTION_FILE;
}
Ok(())
}

/// Whether this build understands data overlay files: always in debug builds,
/// and in release builds only when [`ENABLE_UNSTABLE_DATA_OVERLAY_FILES_ENV`] is set.
fn data_overlay_files_enabled() -> bool {
cfg!(debug_assertions) || std::env::var_os(ENABLE_UNSTABLE_DATA_OVERLAY_FILES_ENV).is_some()
}

/// Clear `flag` from `flags` when its gating feature is not enabled in this
/// build; leave it set otherwise. One call per unstable flag, so support for
/// several unstable features chains cleanly.
fn mark_supported(flags: &mut u64, flag: u64, feature_enabled: bool) {
if !feature_enabled {
*flags &= !flag;
}
}

/// The feature-flag bits this build understands, given whether overlay support
/// is enabled. Split out from [`supported_flags`] so the policy is testable
/// without toggling the build profile or environment.
fn supported_flags_when(overlay_enabled: bool) -> u64 {
let mut supported = FLAG_UNKNOWN - 1;
mark_supported(
&mut supported,
FLAG_UNSTABLE_DATA_OVERLAY_FILES,
overlay_enabled,
);
supported
}

fn supported_flags() -> u64 {
supported_flags_when(data_overlay_files_enabled())
}
Comment thread
wjones127 marked this conversation as resolved.

pub fn can_read_dataset(reader_flags: u64) -> bool {
reader_flags < FLAG_UNKNOWN
reader_flags & !supported_flags() == 0
}

pub fn can_write_dataset(writer_flags: u64) -> bool {
writer_flags < FLAG_UNKNOWN
writer_flags & !supported_flags() == 0
}

pub fn has_deprecated_v2_feature_flag(writer_flags: u64) -> bool {
Expand All @@ -103,6 +161,13 @@ mod tests {
assert!(can_read_dataset(super::FLAG_TABLE_CONFIG));
assert!(can_read_dataset(super::FLAG_BASE_PATHS));
assert!(can_read_dataset(super::FLAG_DISABLE_TRANSACTION_FILE));
// Overlay support is gated on the build profile / env opt-in, so the
// flag is readable exactly when overlays are enabled (see
// test_data_overlay_flag_release_gating for the full policy).
assert_eq!(
can_read_dataset(super::FLAG_UNSTABLE_DATA_OVERLAY_FILES),
data_overlay_files_enabled()
);
assert!(can_read_dataset(
super::FLAG_DELETION_FILES
| super::FLAG_STABLE_ROW_IDS
Expand All @@ -111,6 +176,58 @@ mod tests {
assert!(!can_read_dataset(super::FLAG_UNKNOWN));
}

#[test]
fn test_data_overlay_flag_release_gating() {
// Release default (overlays disabled): the overlay flag is treated as
// unknown so the dataset is refused, while other known flags still pass.
let supported = supported_flags_when(false);
assert_eq!(supported & FLAG_UNSTABLE_DATA_OVERLAY_FILES, 0);
assert_eq!(FLAG_DELETION_FILES & !supported, 0);
assert_ne!(FLAG_UNSTABLE_DATA_OVERLAY_FILES & !supported, 0);
// Enabled (debug or env opt-in): the overlay flag is understood.
let supported = supported_flags_when(true);
assert_eq!(FLAG_UNSTABLE_DATA_OVERLAY_FILES & !supported, 0);
}

#[test]
fn test_apply_feature_flags_sets_overlay_flag() {
use crate::format::overlay::{DataOverlayFile, OverlayCoverage};
use crate::format::{DataFile, DataStorageFormat, Fragment};
use arrow_schema::{Field as ArrowField, Schema as ArrowSchema};
use lance_core::datatypes::Schema;
use roaring::RoaringBitmap;
use std::collections::HashMap;
use std::sync::Arc;

let arrow_schema = ArrowSchema::new(vec![ArrowField::new(
"id",
arrow_schema::DataType::Int64,
false,
)]);
let schema = Schema::try_from(&arrow_schema).unwrap();
let mut fragment = Fragment::new(0);
fragment.overlays = vec![DataOverlayFile {
data_file: DataFile::new_legacy_from_fields("o.lance", vec![0], None),
coverage: OverlayCoverage::dense(RoaringBitmap::from_iter([0u32])),
committed_version: 1,
}];
let mut manifest = Manifest::new(
schema,
Arc::new(vec![fragment]),
DataStorageFormat::default(),
HashMap::new(),
);
apply_feature_flags(&mut manifest, false, false).unwrap();
assert_ne!(
manifest.reader_feature_flags & FLAG_UNSTABLE_DATA_OVERLAY_FILES,
0
);
assert_ne!(
manifest.writer_feature_flags & FLAG_UNSTABLE_DATA_OVERLAY_FILES,
0
);
}

#[test]
fn test_write_check() {
assert!(can_write_dataset(0));
Expand All @@ -120,6 +237,13 @@ mod tests {
assert!(can_write_dataset(super::FLAG_TABLE_CONFIG));
assert!(can_write_dataset(super::FLAG_BASE_PATHS));
assert!(can_write_dataset(super::FLAG_DISABLE_TRANSACTION_FILE));
// Overlay support is gated on the build profile / env opt-in, so the
// flag is writable exactly when overlays are enabled (see
// test_data_overlay_flag_release_gating for the full policy).
assert_eq!(
can_write_dataset(super::FLAG_UNSTABLE_DATA_OVERLAY_FILES),
data_overlay_files_enabled()
);
assert!(can_write_dataset(
super::FLAG_DELETION_FILES
| super::FLAG_STABLE_ROW_IDS
Expand Down
1 change: 1 addition & 0 deletions rust/lance-table/src/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use uuid::Uuid;
mod fragment;
mod index;
mod manifest;
pub mod overlay;
mod transaction;

pub use crate::rowids::version::{
Expand Down
Loading
Loading