Skip to content

feat: check write inputs against the data file schema at a single boundary - #9478

Merged
Xuanwo merged 4 commits into
mainfrom
xuanwo/semantic-type-write-boundary
Sep 23, 2026
Merged

Xuanwo merged 4 commits into
mainfrom
xuanwo/semantic-type-write-boundary

Conversation

@Xuanwo

@Xuanwo Xuanwo commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Part of #7073.

Problem

Arrow JSON input (arrow.json text) must be converted to Lance JSONB before it is encoded, but each dataset write path opted into the conversion on its own. A path that forgot it wrote UTF-8 bytes into a column whose file schema says LargeBinary + lance.json, and the file was only found to be corrupt on read (#7469–#7473 each fixed one such path). Nothing in the file writers noticed: the current-format writers (2.0–2.3) check column presence and nullability, but never compare an array's type or extension with the file schema.

Behavior

  • Every current-format file writer checks each batch (and each write_column array) against the file schema before encoding. A mismatch returns Error::InvalidInput whose source is a FieldTypeMismatch naming the field path and the expected and actual Arrow type and extension. The check reads only types and field metadata, never values.
  • Dataset writes convert Arrow JSON to JSONB (and top-level view arrays to offset arrays, as today) in one place: the current-format data file writer that all dataset data files now go through, including single-fragment creation and data file parts. The per-call-site conversions in the write, append, fragment-create, updater and merge-insert paths are gone.
  • A Lance json field records lance.json whether it was created from arrow.json or lance.json input, so schemas derived directly from caller data match the stored layout.

Tradeoffs

  • The check lives in lance-file rather than the dataset writer because it is the only layer every data file passes through (dataset writes, data file parts, and direct lance-file users), and it owns the file schema that readers trust. The conversion stays in the dataset layer: it is table semantics, and the file layer only rejects input that skipped it.
  • Names and nullability of nested fields are not compared, since encoders address children by position and check nulls against values. Utf8View/BinaryView are accepted where the schema says Utf8/Binary, because the encoders already write them in the offset layout. Blob v2 fields are checked only at the struct level: the encoder receives the descriptor struct from blob preprocessing, not the struct the file schema records, and validates it itself.
  • The full check runs once per writer and again only when a batch's schema differs from the last accepted one; later batches cost a pointer or schema equality check. Measured on the first batch, it adds about 0.05 µs per flat column and 0.3 µs per struct<int, json> column, which is 1–4% of encoding one flat batch and 7–13% of encoding one small nested JSON batch. That cost is paid once per file.
  • write_column receives a bare array, so the column's own extension cannot be checked there; nested extensions can.
  • Two write paths still convert early, because they combine caller data with values read back from storage before writing: partial-fragment merge-insert (interleave) and update_columns (hash-join fallback). They are no longer the only guard.
  • Legacy (V1) files keep their existing schema compatibility check and are not changed.

…ndary

Data file writers now reject any array whose Arrow type or extension
differs from the file schema before encoding, returning a structured
FieldTypeMismatch. Dataset writes convert Arrow JSON to Lance JSONB in the
one current-format data file writer instead of at each call site.
@github-actions github-actions Bot added A-encoding Encoding, IO, file reader/writer enhancement New feature or request labels Sep 22, 2026
Build the expected Arrow fields once per writer, match batch columns by
position before falling back to a name search, and skip batches whose
schema equals the last accepted one.
@Xuanwo
Xuanwo marked this pull request as ready for review September 23, 2026 03:44
FieldTypeMismatch now carries the expected and actual Arrow fields instead
of a separate type-and-extension struct, and the check walks nested
fields without tracking a path stack.
lance-gatekeeper[bot]

This comment was marked as outdated.

@lance-gatekeeper lance-gatekeeper Bot added the K-changes Latest Gatekeeper recommendation requests changes. label Sep 23, 2026
do_write_fragments_impl hands each batch to both the data file writer and
the index seed observers. Convert it once before both, so appending view
or Arrow JSON input to a seeded column no longer fails in the seed writer.
@lance-gatekeeper lance-gatekeeper Bot removed the K-changes Latest Gatekeeper recommendation requests changes. label Sep 23, 2026

@lance-gatekeeper lance-gatekeeper Bot left a comment

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.

✅ Gate recommendation: approve.

The indexed Utf8View append regression is fixed: normalized batches now feed both file encoding and seed observation. The added regression test passes, along with the nested JSON write-path and writer mismatch coverage.

@lance-gatekeeper lance-gatekeeper Bot added the K-approved Latest Gatekeeper recommendation permits acceptance. label Sep 23, 2026
@Xuanwo
Xuanwo merged commit 274b5bf into main Sep 23, 2026
39 checks passed
@Xuanwo
Xuanwo deleted the xuanwo/semantic-type-write-boundary branch September 23, 2026 04:58
Xuanwo added a commit that referenced this pull request Sep 23, 2026
…9479)

Part of #7073. Builds on #9478.

## Problem

Code that computes on JSON decided whether a column holds JSON by
matching `lance.json` over `LargeBinary` itself. The same JSON column
was therefore tokenized as JSON when it came from storage but as plain
text when it arrived as Arrow JSON text (`arrow.json` over `Utf8`), and
the JSON scalar index could not read Arrow JSON text at all. JSONPath
selection was also written twice: `JsonArray::json_path` kept only the
first match, while `json_extract` returns every match as one JSON array.

## Behavior

- `lance_arrow::json` is the one place that encodes and decodes JSONB
and selects JSONPath values. `JsonEncoding::of_field` recognizes a JSON
field in either representation, and `JsonValues` reads its values as
JSONB or as text, whichever the consumer needs, regardless of the
representation they arrived in.
- FTS index builds and unindexed FTS input read JSON documents through
`JsonValues`, so both representations are tokenized as JSON. The JSON
scalar index accepts Arrow JSON text and indexes the same values it
would from JSONB.
- The SQL JSON functions use the shared JSONPath selection.
`JsonArray::json_path` now returns every match of a multi-match path as
one JSON array, the same as `json_extract`.
- Breaking API changes:
`lance_index::scalar::inverted::json::JsonTextStream::new` is replaced
by the fallible `try_new`, which rejects a column that does not hold
JSON, and `jsonb_to_json` is removed in favor of `JsonValues::to_text`.
- No format change, and blob behavior is unchanged. `JsonValues` is an
internal Rust API, not a plugin extension point.

## Tradeoffs

- `dataset/sql.rs`, `lance-datafusion/src/projection.rs` and
`io/exec/projection.rs` do not inspect JSON metadata themselves: SQL
only converts its output to Arrow JSON through `SchemaAdapter`, and the
projections carry field metadata through unchanged. They are left as
they are; choosing the user-facing output encoding belongs to the read
path.
- An FTS index records its tokenizer when it is created, so existing
indices keep theirs. The document-type change only affects new indices
built from Arrow JSON text.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-encoding Encoding, IO, file reader/writer enhancement New feature or request K-approved Latest Gatekeeper recommendation permits acceptance.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants