Skip to content

bug(overlay): struct cell validity is not replaced by a data overlay #9077

Description

@wjones127

Summary

Data overlay resolution cannot change the validity (null-ness) of a struct cell. Overlaying a nullable struct's children updates the child values but silently preserves the base struct's null buffer, so NULL -> non-NULL and non-NULL -> NULL transitions are lost.

This contradicts the overlay contract, which states that a covered offset holding NULL overrides that cell to NULL (see rust/lance/src/dataset/overlay/writer.rs module docs and docs/src/format/table/data_overlay_file.md).

Cause

Overlay resolution is per atomic field, and a struct is recursed through rather than treated as an atomic unit — only its leaves are atomic (rust/lance/src/dataset/overlay.rs:371-379).

At merge time, splice_by_ids destructures the base StructArray and rebuilds it with the base null buffer:

// rust/lance/src/dataset/overlay.rs:566-575
let (fields, mut children, nulls) = structs.clone().into_parts();
children[child_pos] = splice_by_ids(...)?;
Ok(Arc::new(StructArray::try_new_with_length(
    fields, children, nulls, len,   // <- base `nulls`, never the overlay's
)?))

The writer does faithfully encode the struct's validity into the overlay value file; the read path structurally ignores it.

Reproduction

  1. Create a two-row dataset with a nullable struct column, child values [1, 2], struct validity [false, true].
  2. Overlay both rows with child values [10, 20] and struct validity [true, false].
  3. Commit and scan.

Observed: child values [10, 20] (correct), struct validity [false, true] (stale).
Expected: struct validity [true, false].

Options

  1. Represent and merge validity at every struct ancestor — either give struct parents their own atomic-field entry carrying only validity, or have the merge recompute parent nulls from covered children. Read-path change, possibly a format change.
  2. Reject nullable struct ancestors at the writer until (1) lands — fail closed rather than returning wrong data.

Interim mitigation

Tracking this separately from #8761, which adds OverlayWriter. That PR takes approach (2) as a stopgap so the writer cannot produce an overlay whose semantics the read path will not honor. This issue tracks doing it properly.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions