refactor: read JSON through one accessor in the JSON semantic module - #9479
Merged
Merged
Conversation
…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.
JSON encoding detection, JSONB/text conversion and JSONPath selection now live in lance_arrow::json. FTS and the JSON scalar index read values through JsonValues, so they treat stored JSONB and Arrow JSON text alike.
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.
…ic-type-json-accessor
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.
…ic-type-json-accessor
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.
…ic-type-json-accessor
Base automatically changed from
xuanwo/semantic-type-write-boundary
to
main
September 23, 2026 04:58
Contributor
There was a problem hiding this comment.
✅ Gate recommendation: approve.
The shared JSON accessor is preferable to per-consumer metadata branching: FTS, JSON scalar indexing, and JSONPath evaluation apply the same representation semantics to stored JSONB and Arrow JSON text, while existing index tokenizer metadata remains intentionally preserved. The affected accessor, UDF, and index paths have focused regression coverage, including multi-match JSONPath selection.
BubbleCal
approved these changes
Sep 23, 2026
sbrunk
added a commit
to sbrunk/lance
that referenced
this pull request
Sep 25, 2026
The flat `combined_fields` path wraps its input in one `JsonTextStream` per JSON target column, so it depends on that stream declaring the schema its batches actually have. Upstream lance-format#9479 made `JsonTextStream` derive one schema from its input and use it for both `schema()` and the batches, which fixes the mismatch this branch originally patched. What remains are regression tests for the caller that the mismatch broke. Two `combined_fields` scans over `arrow.json` columns with mixed index coverage, so the plan carries a flat child: one over two JSON columns, which needs name resolution to survive wrapping the stream twice, and one mixing a JSON column with a plain text column, where the JSON column is not at position 0 so a positional read would tokenize the wrong data. Both fail if `JsonTextStream::schema()` lists only the JSON column.
sbrunk
added a commit
to sbrunk/lance
that referenced
this pull request
Sep 25, 2026
The flat `combined_fields` path wraps its input in one `JsonTextStream` per JSON target column, so it depends on that stream declaring the schema its batches actually have. Upstream lance-format#9479 made `JsonTextStream` derive one schema from its input and use it for both `schema()` and the batches, which fixes the mismatch this branch originally patched. What remains are regression tests for the caller that the mismatch broke. Two `combined_fields` scans over `arrow.json` columns with mixed index coverage, so the plan carries a flat child: one over two JSON columns, which needs name resolution to survive wrapping the stream twice, and one mixing a JSON column with a plain text column, where the JSON column is not at position 0 so a positional read would tokenize the wrong data. Both fail if `JsonTextStream::schema()` lists only the JSON column.
sbrunk
added a commit
to sbrunk/lance
that referenced
this pull request
Sep 25, 2026
The flat `combined_fields` path wraps its input in one `JsonTextStream` per JSON target column, so it depends on that stream declaring the schema its batches actually have. Upstream lance-format#9479 made `JsonTextStream` derive one schema from its input and use it for both `schema()` and the batches, which fixes the mismatch this branch originally patched. What remains are regression tests for the caller that the mismatch broke. Two `combined_fields` scans over `arrow.json` columns with mixed index coverage, so the plan carries a flat child: one over two JSON columns, which needs name resolution to survive wrapping the stream twice, and one mixing a JSON column with a plain text column, where the JSON column is not at position 0 so a positional read would tokenize the wrong data. Both fail if `JsonTextStream::schema()` lists only the JSON column.
sbrunk
added a commit
to sbrunk/lance
that referenced
this pull request
Sep 26, 2026
The flat `combined_fields` path wraps its input in one `JsonTextStream` per JSON target column, so it depends on that stream declaring the schema its batches actually have. Upstream lance-format#9479 made `JsonTextStream` derive one schema from its input and use it for both `schema()` and the batches, which fixes the mismatch this branch originally patched. What remains are regression tests for the caller that the mismatch broke. Two `combined_fields` scans over `arrow.json` columns with mixed index coverage, so the plan carries a flat child: one over two JSON columns, which needs name resolution to survive wrapping the stream twice, and one mixing a JSON column with a plain text column, where the JSON column is not at position 0 so a positional read would tokenize the wrong data. Both fail if `JsonTextStream::schema()` lists only the JSON column.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Part of #7073. Builds on #9478.
Problem
Code that computes on JSON decided whether a column holds JSON by matching
lance.jsonoverLargeBinaryitself. 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.jsonoverUtf8), and the JSON scalar index could not read Arrow JSON text at all. JSONPath selection was also written twice:JsonArray::json_pathkept only the first match, whilejson_extractreturns every match as one JSON array.Behavior
lance_arrow::jsonis the one place that encodes and decodes JSONB and selects JSONPath values.JsonEncoding::of_fieldrecognizes a JSON field in either representation, andJsonValuesreads its values as JSONB or as text, whichever the consumer needs, regardless of the representation they arrived in.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.JsonArray::json_pathnow returns every match of a multi-match path as one JSON array, the same asjson_extract.lance_index::scalar::inverted::json::JsonTextStream::newis replaced by the fallibletry_new, which rejects a column that does not hold JSON, andjsonb_to_jsonis removed in favor ofJsonValues::to_text.JsonValuesis an internal Rust API, not a plugin extension point.Tradeoffs
dataset/sql.rs,lance-datafusion/src/projection.rsandio/exec/projection.rsdo not inspect JSON metadata themselves: SQL only converts its output to Arrow JSON throughSchemaAdapter, 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.