feat(format): add target_data_type to JsonIndexDetails

[wjones127]

A JSON value has no fixed type, so a JSON index picks one type for the path it indexes and trains its target index on that. JsonIndexDetails does not record which type it picked, so the details are not enough to rebuild an equivalent index. A rebuild re-infers the type from the first non-null value it sees, falling back to string when everything is null — so a rebuild over different rows, or a build split across workers each training over different fragments, can end up with different types and nothing to reconcile them.

This adds JsonIndexDetails.target_data_type, an enum covering every JSON value type. JSON_TARGET_DATA_TYPE_UNSPECIFIED means the type was not recorded, and those indices keep loading, querying and compacting exactly as before.

The field also carries a requirement on readers: an implementation that rewrites these details without retraining the target index must copy the value through unchanged, including one it does not recognize. Otherwise a variant added by a later revision is erased by any older reader that touches the index.

Not included

This is the specification only, as protos/AGENTS.md and the gatekeeper's note on #9261 ask. The three JsonIndexDetails literals in json.rs name the new field so the build stays green; they record UNSPECIFIED, which is the value the format already assigns to details written before the field existed, so nothing changes at runtime. The writers and readers that populate it are in #9304, stacked on this PR.

No API for building an index from details — lance has no such entry point for scalar indices, and adding one is a larger design question. #4628 was considered and left out; that reasoning, and the encodings rejected for this field, are in the decision record.

Part of #9256

🤖 Generated with Claude Code

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[github-actions]

ACTION NEEDED
Lance follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

For details on the error please inspect the "PR Title Check" action.

[github-actions]

Important

Format specification vote

This PR modifies the Lance format specification, so it requires 3 binding +1 votes from PMC members (excluding the proposer), at least one of them on the latest commit, and a minimum 72-hour voting period, weekends excluded, before it can merge. Vote by approving this PR (+1) or requesting changes (−1, a veto). See the voting process.

Approvals carry over across pushes, so a rebase or a typo fix does not send everyone back to re-vote. Whoever approves the latest commit is vouching that nothing substantive has changed since the earlier approvals; if something has, ask for fresh votes.

Status: ❌ Blocked — 1 of 3 required approvals

Approvals	@jackye1995 (1/3)
Latest commit approved by	@jackye1995
Vetoes	none
Voting period	elapsed — ended Mon 2026-09-21 16:21 UTC (09:21 PDT)

_{Updated automatically by the format-spec vote gate, which re-checks every 15 minutes — just voted? Re-check now (press Run workflow; leave the input blank to re-check every open format PR). A PMC member may apply the format-waived label to waive the vote for a trivial edit (typo, wording, formatting).}

[wjones127]

Both points taken; pushed as docs(format): clarify UNSPECIFIED recovery order and completeness scope.

On UNSPECIFIED and re-inference — agreed, and the old wording contradicted the stacked implementation. UNSPECIFIED now states an explicit recovery order: a reader asks the target index first, because some target types retain the Arrow type they were trained on and that answer is authoritative; first-non-null/UTF8 inference is the last resort, only when the target cannot answer. The text also says an implementation that recovers the type authoritatively while rewriting the details should record it, so the index stops being ambiguous. The format docs page carries the same ordering.

On the completeness claim — agreed, and scoped. The field comment now says recording the type "completes the description of the extracted values only. These details still do not record the target index's format version, so they are not a general, lossless description of the index and must not be read as one."

On the target index version — I re-evaluated folding #4628 into this PR, and it stays out, but your comment surfaced something worse than a documentation gap, so recording it here.

The version problem is not only that JsonIndexDetails lacks the field. A JSON index discards its target's version and stamps its own: CreatedIndex.index_version is JSON_INDEX_VERSION (0) at json.rs:144, :183 and :979, while target_created.index_version is dropped. On the read side IndexDetails::index_version() (index/scalar.rs:326) resolves the plugin from the outer type_url and returns JsonIndexPlugin::version() = 0, never recursing into target_details. So unsupported_index_version (index.rs:2734) — which every JSON-wrapped index does reach, via index_is_usable on manifest load, optimize_indices, and compaction planning — is fed a version that is structurally blind to the target.

That already misreports today: FMINDEX_INDEX_VERSION = 10, LABEL_LIST_INDEX_VERSION = 1, and fm is a supported JSON target, so a JSON-wrapped fm index commits with index_version = 0 where the bare one commits 10. And it fails open on a future bump: a newer build's JSON-wrapped btree v1 would be judged usable by a v0 build and handed v1 files, where the unwrapped equivalent would be cleanly hidden from queries, skipped by optimize, and held out of the compaction plan.

Adding target_index_version to this message does not fix that, because nothing compares such a field. The fix is at the two ends of the gate that already exists and needs no format change: stamp max(JSON_INDEX_VERSION, target_created.index_version) at the three writer sites, and have IndexDetails::index_version() return max(json_plugin.version(), target_plugin.version()). There is direct precedent in that same function — it already collapses vector subtypes onto IndexType::max_vector_version() because VectorIndexDetails does not record its subtype either.

Since it changes what is written to the manifest, it needs its own compatibility analysis and its own PR rather than riding along on a format vote. Full write-up, including the retraction of the earlier (wrong) reasoning for leaving #4628 out, is in the decision record.

[wjones127]

Good catch — that was a real contradiction, not just loose wording. For a legacy 0 whose target reports INT64, the UNSPECIFIED paragraph said write 2 and the field paragraph said keep 0. Fixed in bf9429d; both paragraphs now name the exception.

Field 3's rule is now scoped to values that actually record a type:

An implementation that rewrites these details without retraining the target index — compacting or updating it — must preserve a recorded type exactly. Every value other than JSON_TARGET_DATA_TYPE_UNSPECIFIED is copied through unchanged, including one the implementation does not recognize […] JSON_TARGET_DATA_TYPE_UNSPECIFIED is the sole exception, because it records no type to preserve.

And UNSPECIFIED states the carve-out from its own side, with one addition worth flagging since it tightens the contract rather than just reconciling it:

[…] an implementation rewriting these details may replace UNSPECIFIED with a type it recovered authoritatively, and should, so the index stops being ambiguous. It must not replace it with an inferred type, which would persist a row-dependent guess as though it were recorded fact.

That last sentence matters for the case you raised earlier: a compaction sees whatever fragments it was given, so letting a rewrite persist an inferred type would bake the disjoint-subset inconsistency into the details permanently — worse than leaving UNSPECIFIED, which at least stays honest about not knowing. Inference at training time is unaffected; there the inferred type is the type the index was actually trained on, so recording it is recording fact.

No implementation change needed — this is what #9304 already does, and both halves are pinned by tests: test_json_legacy_details_without_target_data_type (btree upgrades on remap, bitmap stays UNSPECIFIED), test_json_update_records_target_data_type_for_legacy_details, and test_json_unknown_target_data_type_survives_remap / ..._update (raw 99 preserved through both rewrite paths).

[lance-gatekeeper]

✅ Gate recommendation: approve.

The author’s follow-up resolves the remaining contract contradiction: UNSPECIFIED is now the sole upgrade exception, only an authoritatively recovered type may replace it, and every recorded nonzero value—including future enum variants—must remain unchanged. The additive protobuf shape remains readable across versions, and the stacked implementation exercises both the legacy-upgrade and raw-value preservation paths.

[jackye1995]

looks good to me!