Skip to content

fix(index): convert JSON values during index updates - #8404

Merged
Xuanwo merged 3 commits into
mainfrom
gatekeeper/fix-5177-1
Aug 8, 2026
Merged

fix(index): convert JSON values during index updates#8404
Xuanwo merged 3 commits into
mainfrom
gatekeeper/fix-5177-1

Conversation

@lance-gatefixer

Copy link
Copy Markdown
Contributor

Summary

  • extract and type-convert JSON path values before forwarding scalar index updates
  • preserve target ordering on the extracted value and map explicit JSON nulls to Arrow nulls
  • add a three-fragment regression for JSON B-tree index optimization

Root cause

Initial JSON index training converted the selected path from raw JSONB into its inferred scalar type, but the JSON index update path forwarded newly appended raw LargeBinary JSON directly to the target B-tree. Optimizing the index then tried to merge that raw stream with the existing typed Int64 index data.

Validation

  • cargo test -p lance-index scalar::json::tests -- --nocapture
  • cargo test -p lance test_optimize_json_btree_index -- --nocapture
  • cargo fmt --all -- --check
  • cargo clippy --all --tests --benches -- -D warnings

Fixes #5177

@github-actions github-actions Bot added A-index Vector index, linalg, tokenizer bug Something isn't working labels Aug 7, 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: request changes.

JSON optimization needs to preserve the trained target type and JSON-wrapper semantics across both merge and append-rebuild paths, while keeping conversion streaming. A viable revision would convert each batch against the loaded target's expected type, retain the JSON path and target parameters when deriving a rebuild, and apply target ordering after extraction.

Comment thread rust/lance-index/src/scalar/json.rs Outdated
) -> Result<CreatedIndex> {
let target_criteria = self.target_index.update_criteria().data_criteria;
let (new_data, inferred_type) =
JsonIndexPlugin::extract_json_with_type_info(new_data, self.path.clone()).await?;

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.

This newly routes every update through helpers that drain the complete delta into all_batches, then into converted_batches, before sort_stream_by_value can start its spillable SortExec. Large append optimization is therefore O(delta) memory and can OOM before spilling helps. Make extraction/conversion a batch-by-batch stream; using the target's known type removes the need for whole-delta inference.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 7c0b407. JSON extraction and conversion now map batches lazily; a regression verifies that the first converted batch is returned without polling the next input batch.

Comment thread rust/lance-index/src/scalar/json.rs Outdated
let target_criteria = self.target_index.update_criteria().data_criteria;
let (new_data, inferred_type) =
JsonIndexPlugin::extract_json_with_type_info(new_data, self.path.clone()).await?;
let new_data = JsonIndexPlugin::convert_stream_by_type(new_data, inferred_type).await?;

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.

This converts the delta using a type inferred only from new fragments, but the loaded target index's existing value type is the update contract. A null/missing-only delta defaults to Utf8, and an integer-only delta for a Float64 index becomes Int64; BTreeIndex::merge_segments rejects both. Use the stored target value type for conversion (and type tags for per-value validation/null handling), so compatible updates retain the established schema and incompatible drift fails with the path and expected/actual types.

Reproducer

I ran a temporary rstest that trained the initial JSON B-tree and called index.update(...) with these cases:

#[case::all_null_delta(
    &[r#"{"v": 1}"#],
    &[r#"{"v": null}"#, r#"{"other": 2}"#],
    "Utf8 does not match segment value type Int64"
)]
#[case::integer_delta_for_float_index(
    &[r#"{"v": 1.5}"#],
    &[r#"{"v": 2}"#],
    "Int64 does not match segment value type Float64"
)]
CARGO_TARGET_DIR=/home/agent/tmp/pr8404-impl-target cargo test -p lance-index review_repro_update_infers_type_from_delta -- --nocapture

Both cases reproduced the asserted B-tree type mismatch; these updates should optimize successfully by converting to the stored Int64/Float64 type.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 7c0b407. Updates now convert against the B-tree's stored data type, preserve null and missing values, allow Int64 values for Float64 targets, and report the JSON path plus expected and actual types on incompatible drift. Both reproducer cases have regression coverage.

let target_criteria = self.target_index.update_criteria();
UpdateCriteria {
requires_old_data: target_criteria.requires_old_data,
data_criteria: json_scan_criteria(&target_criteria.data_criteria),

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.

update_criteria is also consumed by rebuild_scalar_segment, so this raw-unordered scan contract reaches OptimizeOptions::append(). That path still gets direct B-tree parameters from JsonIndex::derive_index_params and passes this preprocessed raw JSON stream into B-tree training, producing BTreeIndexDetails beside existing JsonIndexDetails; the logical index then refuses to load. derive_index_params needs to preserve the JSON wrapper, path, and target parameters so rebuilds run the JSON trainer and post-extraction ordering.

Reproducer

I ran a temporary integration test with:

let initial = json_batch(vec![r#"{"val": 1000}"#]);
let appended = json_batch(vec![
    r#"{"val": 3000}"#,
    r#"{"val": 1000}"#,
    r#"{"val": 2000}"#,
]);
dataset
    .optimize_indices(&OptimizeOptions::append())
    .await
    .unwrap();

let error = dataset
    .scan()
    .filter("json_get_int(json, 'val') >= 2000")
    .unwrap()
    .try_into_batch()
    .await
    .unwrap_err();
assert!(error.to_string().contains("mixes incompatible segment types"));
CARGO_TARGET_DIR=/home/agent/tmp/pr8404-impl-target cargo test -p lance review_repro_append_json_btree_index -- --nocapture

It produced one JsonIndexDetails segment and one BTreeIndexDetails segment, then failed the query with Scalar index 'json_idx' on column 'json' mixes incompatible segment types.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 7c0b407. JsonIndex::derive_index_params now retains the JSON wrapper, path, target type, and target parameters; merge and append-rebuild integration cases both pass.

@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: request changes.

The direct-update path is fixed, but append rebuilds still need to preserve the loaded JSON target learned value type, not only the wrapper and construction parameters. Carry that type through derived rebuild parameters and convert against it so every segment remains query-compatible.

target_index_type: target_params.index_type,
target_index_parameters: target_params.params,
path: self.path.clone(),
};

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.

These derived parameters still drop the learned target value type. OptimizeOptions::append() rebuilds the new segment from them, so a Float64 base followed by an integer-only delta is inferred as Int64. Both segments still load as JSON, but the Float64 query is evaluated against Int64 B-tree page statistics and silently prunes the appended row. Include the trained target type in the derived JSON parameters (optionally absent for an initial build) and make rebuild training convert against it instead of inferring per segment.

Reproducer

I ran a disposable end-to-end regression that trained a JSON B-tree from {"val":1.5}, appended {"val":2}, and completed OptimizeOptions::append(). The same json_get_float equality predicate on path val for 2.0 returned one row with scalar indexes disabled but zero rows with the index enabled; the indexed assertion failed with left: 0, right: 1.

CARGO_TARGET_DIR=/home/agent/tmp/pr8404-followup-target cargo test -p lance test_disposable_optimize_append_float_json_with_integer_delta -- --nocapture

The command exited 101 with 0 passed and 1 failed.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 61fccdf. Derived JSON parameters now include the optional learned target data type, and rebuild training converts each new segment against that type instead of re-inferring it. The Float64-base/Int64-delta append regression now verifies indexed results against an index-disabled baseline.

@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 learned JSON B-tree type is now preserved through append rebuild parameters, so direct updates and new segments share the same schema while initial builds retain inference. The prior Float64-base/integer-append false negative now passes against an index-disabled baseline, and conversion remains streaming.

@Xuanwo
Xuanwo merged commit 3f84339 into main Aug 8, 2026
40 checks passed
@Xuanwo
Xuanwo deleted the gatekeeper/fix-5177-1 branch August 8, 2026 10:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-index Vector index, linalg, tokenizer bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

JSON BTree index fails on optimize_indices

1 participant