JsonIndexDetails cannot represent target_data_type, so details are insufficient to rebuild a JSON index

[wjones127]

Summary

JsonIndexDetails cannot represent the target data type, so the persisted details are not sufficient to rebuild an equivalent JSON index.

message JsonIndexDetails {
  string path = 1;
  google.protobuf.Any target_details = 2;
}

(protos/index.proto)

target_details for the common targets is an empty message — BTreeIndexDetails {} and BitmapIndexDetails {} (protos/index_old.proto). So the entire persisted payload for a JSON index is a path plus the target's type identity.

Meanwhile JsonIndexParameters (rust/lance-index/src/scalar/json.rs) carries four fields:

struct JsonIndexParameters {
    target_index_type: String,
    target_index_parameters: Option<String>,
    target_data_type: Option<JsonIndexTargetType>,
    path: String,
}

target_data_type has no home in the details. Details are a strict subset of params here.

Why it matters

When target_data_type is omitted, JsonIndexPlugin infers the type from the data — infer_type_from_batch returns on the first non-null value's type tag and falls back to Utf8 when everything is null. That makes a rebuild driven purely by details non-deterministic with respect to the data it sees:

• A rebuild over a different row set can infer a different type than the original index.
• A build that is split across workers, each training over a disjoint subset of fragments, can have each worker infer independently. A sparse field is enough to diverge: one subset sees {"v": 1} and infers Int64, another is entirely null at that path and falls back to Utf8.

There is no cross-worker or cross-build coordination on the create path, so nothing reconciles the disagreement.

Note the type is recoverable from a loaded index — JsonIndex::derive_index_params() reconstructs the full parameters including target_data_type via target_index.training_data_type(). The gap is specifically in the persisted details, which is the representation a downstream system would naturally carry and pass through.

Request

Add target_data_type to JsonIndexDetails so the details are a faithful, lossless description of the index, and a rebuild from details reproduces the original schema rather than re-inferring it.

This is the same class as #4628 (JSON index should store the target index version in the details) — the details message is currently missing information required to reconstruct the index.

Related: #5311 (make JsonIndexParameters constructible), which covers the params side of the same problem.

[wjones127]

Decision

Add a single field to JsonIndexDetails, encoded as a bare proto3 enum:

enum JsonTargetDataType {
  JSON_TARGET_DATA_TYPE_UNSPECIFIED  = 0;
  JSON_TARGET_DATA_TYPE_BOOLEAN      = 1;
  JSON_TARGET_DATA_TYPE_INT64        = 2;
  JSON_TARGET_DATA_TYPE_FLOAT64      = 3;
  JSON_TARGET_DATA_TYPE_UTF8         = 4;
  JSON_TARGET_DATA_TYPE_LARGE_BINARY = 5;
}

message JsonIndexDetails {
  string path = 1;
  google.protobuf.Any target_details = 2;
  JsonTargetDataType target_data_type = 3;
}

UNSPECIFIED means the index was built before the field existed: the type is not recorded and must be re-inferred from the data. It is never written by current builds, and it is the single unambiguous "absent" state.

Implementation shape:

• JsonIndex carries the raw enum value, parsed from details in load_index. remap and update copy it forward verbatim rather than re-deriving it; train_index writes the target_type it already computes, so fresh builds record the type even when it was inferred.
• derive_index_params sources from the carried value.
• details_as_json surfaces the field.
• JsonIndex::training_data_type() is unchanged, so update on a non-BTree target keeps erroring exactly as today.

Status

Settled. Implementation in progress. Single PR carrying the proto change, a new docs/src/format/index/scalar/json.md, the implementation, and tests.

Options and criteria

Criteria, in the order they ended up mattering: no ambiguity about what "unspecified" means; no coupling of a stable persisted contract to an encoding we expect to replace; room for a parameterized type later without a wire break; and cost of a future format vote.

• Bare enum (chosen). Exactly one absent state. No coupling. A later parameterized type — e.g. Decimal128(precision, scale) from bug: JSON integers above i64::MAX break JSON index creation and typed reads; numbers above u64::MAX are silently truncated to f64 #8807 — is still wire-compatible, either as a sibling field or by folding field 3 into a oneof, which protobuf documents as safe for a single field that keeps its number and type.

Rejected, with the reason and when it was tried

• string holding a Lance LogicalType (first choice, reversed during design). Attractive because "decimal:128:38:9" would absorb bug: JSON integers above i64::MAX break JSON index creation and typed reads; numbers above u64::MAX are silently truncated to f64 #8807 with no format change, and because target_index_type beside it is already a registry-name string. Rejected on three grounds. (1) It couples a stable index contract to the stringly-typed schema encoding, which is expected to be replaced by an enum in a schema V2; field 3 could never be reinterpreted afterwards. (2) The LogicalType alphabet is wider and lossier than the fact being stored — "blob", "json" and "large_binary" all decode to LargeBinary, Utf8View collapses into "string" — so a reader performs a two-step lossy conversion with an error path for ~25 unreachable values, to recover a 5-valued fact. (3) The argument that it saves a future PMC vote does not hold: any new target type is a feature carrying its own format-doc change and its own vote regardless of how field 3 is encoded.

• A JsonTargetDataType message wrapping a nested Kind enum. Tried in order to keep a future DecimalParams structurally adjacent to the kind. Rejected because it creates two ways to express "unknown" — field 3 absent, or present holding KIND_UNSPECIFIED — resolvable only by the convention "never write UNSPECIFIED", and because it buys no validity guarantee: kind = DECIMAL128 with the params sub-message absent is exactly as representable as the sibling-field equivalent. Its remaining advantage was proto locality, which did not justify the ambiguity.

• Folding in JSON index should store the target index version in the details #4628 (store the target index version in details). Tried because it is the same message and the same class of gap, and one proto revision is cheaper than two. Rejected after checking the consumer: the only live reader of a segment's index_version is the label_list compatibility check at rust/lance/src/index/scalar/label_list.rs:34, and its caller gates on type_url.ends_with("LabelListIndexDetails") (rust/lance/src/index/scalar.rs:592), so a JSON-wrapped index never reaches it. The field would have had no reader at all. JSON index should store the target index version in the details #4628 stays open.

• Sourcing the type at the remap/update call sites via target_index.training_data_type(). Rejected: the trait default returns None (rust/lance-index-core/src/scalar.rs:631) and BTree is the only implementor (rust/lance-index/src/scalar/btree.rs:2395). Every other target — bitmap, label_list, zonemap, bloom filter, ngram, FTS, FM-index, R-tree — would still write details with no type, so compaction would keep losing it for exactly the cases the issue is about. Hence carrying the value on JsonIndex instead.

• Adding training_data_type() overrides to BitmapIndex and ZoneMapIndex (both already hold the type) as an alternative to the format change. Rejected as a substitute, because it cannot serve a builder that has never opened the index, which is the case in the Request above. Noted as a worthwhile independent change: it would also make JsonIndex::update work on those targets instead of returning not_supported at json.rs:155. Deliberately not done here — the guard is load-bearing, since BitmapIndex::update takes its value_type from the incoming stream and merges old keys by OrderableScalarValue comparison, so a type mismatch corrupts rather than errors.

• Shipping a functional reader in this PR. JsonQueryParser::is_valid_reference (json.rs:401) derives the query value type from the UDF name alone and ignores the index's recorded type entirely — it is the natural first consumer. Left out to keep a format-vote PR free of query-routing changes, and because it overlaps bug: JSON index on a nested path is matched by typed accessors that cannot evaluate it, so indexed and unindexed results disagree #9257.

Open questions

• Whether #8807's fix will need UInt64 and/or Decimal target types. If so they are additive here: a new enum value, plus a sibling params field or a oneof migration for the parameterized case.
• remap_index (rust/lance/src/index.rs:1215-1320) has no version gate, unlike optimize_indices (index.rs:2383), and an index this build cannot open is dropped during compaction with a warning rather than erroring (index.rs:1255). Out of scope here; noted because it is what determines whether a version bump would actually protect a newer index from an older writer.

[wjones127]

Decision

Re-evaluating whether to fold #4628 (record the target index format version) into the JsonIndexDetails change, prompted by a format-gate review comment on #9303.

Outcome: still not folded in — but the reason recorded previously was wrong, and a real bug was found in the process.

Status

Settled. #9303 keeps its scope. A separate bug is worth filing for the version gap described below; it needs no format change.

Options and criteria

Criteria: does the field have a reader; does recording it fix a real failure; can the failure be fixed more cheaply; does the cost of a second format revision later outweigh doing it now.

Rejected, with the reason and when it was tried

• Folding in JSON index should store the target index version in the details #4628. Rejected again, but the previously recorded reason was factually wrong and is retracted. The old note said the only live reader of index_version is the label_list check at rust/lance/src/index/scalar/label_list.rs:34, gated behind a LabelListIndexDetails type_url test at rust/lance/src/index/scalar.rs:592, so a JSON-wrapped index never reaches it and the field would have no reader. That is false. unsupported_index_version (rust/lance/src/index.rs:2734) is a generic reader that every JSON-wrapped index passes through on manifest load, on optimize_indices (index.rs:2383), and on compaction planning (dataset/optimize.rs:2242), via index_is_usable (index.rs:2753) and load_indices (index.rs:1903).

  It is rejected now for a different reason: JSON index should store the target index version in the details #4628 as specified does not fix the bug it implies. Adding a target_index_version field to JsonIndexDetails only helps if something compares it against the target plugin's version(), and no such comparison exists or is proposed. The field would be written and never read — the old conclusion, reached correctly this time.

The real bug this uncovered (not fixed here)

A JSON index discards its target's version and stamps its own. At rust/lance-index/src/scalar/json.rs:144 (remap), :183 (update) and :979 (train), CreatedIndex.index_version is set to JSON_INDEX_VERSION (0) while target_created.index_version is dropped. On the reading side, IndexDetails::index_version() (rust/lance/src/index/scalar.rs:326) resolves the plugin by the outer type_url and returns JsonIndexPlugin::version() = 0, never recursing into target_details.

Consequences, present tense:

• FMINDEX_INDEX_VERSION = 10 (fmindex.rs:65) and LABEL_LIST_INDEX_VERSION = 1 (label_list.rs:61). fm is a supported JSON target (scalar::json::tests::test_json_index_preserves_row_addresses::case_2_fm). A JSON-wrapped fm index is therefore committed to the manifest with index_version = 0 where the bare equivalent commits 10. The manifest already records a false version for these indices.
• Forward compatibility fails open. If a later build bumps BTREE_INDEX_VERSION (btree.rs:96) and writes a JSON-wrapped btree, an older build computes max_supported = 0, sees 0 > 0 is false, judges the index usable, and hands v1 files to its v0 BTreeIndexPlugin::load_index. The unwrapped equivalent would be cleanly hidden from queries, skipped by optimize, and held out of the compaction plan. Wrapping an index in JSON currently defeats the version-compatibility gate for that index.
• lance-namespace-impls/src/dir.rs:4415 reports index_version: 0 for every JSON index through the external namespace API.

The fix needs no format change, which is why it does not belong in #9303:

• writer: stamp max(JSON_INDEX_VERSION, target_created.index_version) at the three json.rs sites instead of discarding the target's value;
• reader: have IndexDetails::index_version() return max(json_plugin.version(), target_plugin.version()) for JsonIndexDetails.

There is direct precedent in the same function: index_version() already collapses all vector subtypes onto IndexType::max_vector_version() (rust/lance-index-core/src/lib.rs:266) for exactly this reason — VectorIndexDetails does not record its concrete subtype, so a safe monotone upper bound is used instead. The JSON wrapper is the same shape of problem.

This changes what is written to the manifest, so it needs its own compatibility analysis and its own PR.

Also applied to #9303 from the same review

• JSON_TARGET_DATA_TYPE_UNSPECIFIED no longer says the type "must be inferred by decoding the data again". As a normative rule that was wrong: some targets retain the Arrow type they were trained on, and an implementation following that sentence would re-infer from a different row set and could pick a different type — the inconsistency the field exists to prevent. The recovery order is now explicit: ask the target first, infer only as a last resort.
• The claim that recording the type makes these details "a complete description of the index" is scoped to the extracted values, since the target format version is absent.

Open questions

• IndexType::version() (rust/lance-index-core/src/lib.rs:213-243) is out of sync with the plugin constants it mirrors: it reports LabelList => 0 and Fm => 0 while the plugin constants are 1 and 10. unsupported_index_version uses the plugin values so this is not the live gate, but rust/lance/src/index/append.rs:947 does use IndexType::version(). Unverified whether that is reachable with a stale value; worth a look when the version fix is written.
• IndexType has no Json variant — JsonIndex::index_type() returns IndexType::Scalar (json.rs:84), with its own TODO about list_indices misreporting.