PR #286 (issue #223) changed MilvusFieldKind::index_type to an Option so that the geo column could be materialised without a scalar index — a deliberate exception, taken because Milvus v2.6.19's RTREE prunes with a bounding box smaller than the great-circle cap and silently loses in-radius rows.
The exception is backstopped by only_geo_is_a_deliberately_unindexed_column. That backstop enumerates the field types it checks from a hand-written array, so a schema type that is added to the mapping and not to the array can be materialised with index_type: None, skipped by the index plan, and nothing fires.
Reproduction
Mutation, run against master e99d2b5:
map a new schema type to ("VarChar", None) -> entire suite green
Eight sibling mutations in the same campaign were all killed — float, bool and multivalued-keyword each losing their index; geo getting the defective RTREE back; the dragonfly and valkey geo guards deleted; both refusals widened beyond geo. This one survived.
Why it matters
This is the failure mode #286 exists to prevent, one level up. A field type that is materialised as a column but never indexed produces a full scan rather than an index lookup on every filtered query — the same defect as issue #218, which was filed because Milvus was creating no scalar indexes at all and every filtered query was understating performance by 1.6–2.2×. Recall is unaffected, so nothing in the test suite notices.
The current tree is correct — every non-geo field carries Some(index), verified by mutation. This is a guard that will not catch the next occurrence, not a live defect.
A related correction to the record
PR #286's rework described the #218 invariant as having become "strictly stronger" — from "a column is indexed" to "indexed iff the mapping names an index". That is not right, and the code's own comment says so correctly: the two are incomparable. (column: Some, index: None) was previously unrepresentable and is now a legal state. The invariant did not weaken in practice, because the hand-written array covers every type that exists today; it weakened in principle, and the array is what holds the line.
Suggested direction
Derive the checked set from the mapping itself rather than from a parallel list — iterate every variant the mapping can produce and assert each either carries an index or appears in a small, explicitly-reasoned exception set. Then adding a type without an index is a test failure that names the type, instead of silence.
If that is awkward, the cheaper version is an exhaustive match over the schema-type enum inside the test, so a new variant fails to compile until someone decides which side it belongs on. That is the same shape as the member_set_is_pinned guard added in #264 — make the omission a build error rather than a passing test.
Provenance
Found by an adversarial mutation campaign against merged master e99d2b5 (9 mutations, 8 killed, this one survived). The campaign was reviewing PR #286 and reported after it merged; this is the one finding that is live rather than retrospective. Not introduced by #286 in the sense that no field is currently mis-mapped — the gap is in what the guard can see.
PR #286 (issue #223) changed
MilvusFieldKind::index_typeto anOptionso that the geo column could be materialised without a scalar index — a deliberate exception, taken because Milvus v2.6.19's RTREE prunes with a bounding box smaller than the great-circle cap and silently loses in-radius rows.The exception is backstopped by
only_geo_is_a_deliberately_unindexed_column. That backstop enumerates the field types it checks from a hand-written array, so a schema type that is added to the mapping and not to the array can be materialised withindex_type: None, skipped by the index plan, and nothing fires.Reproduction
Mutation, run against master
e99d2b5:Eight sibling mutations in the same campaign were all killed — float, bool and multivalued-keyword each losing their index; geo getting the defective RTREE back; the dragonfly and valkey geo guards deleted; both refusals widened beyond geo. This one survived.
Why it matters
This is the failure mode #286 exists to prevent, one level up. A field type that is materialised as a column but never indexed produces a full scan rather than an index lookup on every filtered query — the same defect as issue #218, which was filed because Milvus was creating no scalar indexes at all and every filtered query was understating performance by 1.6–2.2×. Recall is unaffected, so nothing in the test suite notices.
The current tree is correct — every non-geo field carries
Some(index), verified by mutation. This is a guard that will not catch the next occurrence, not a live defect.A related correction to the record
PR #286's rework described the #218 invariant as having become "strictly stronger" — from "a column is indexed" to "indexed iff the mapping names an index". That is not right, and the code's own comment says so correctly: the two are incomparable.
(column: Some, index: None)was previously unrepresentable and is now a legal state. The invariant did not weaken in practice, because the hand-written array covers every type that exists today; it weakened in principle, and the array is what holds the line.Suggested direction
Derive the checked set from the mapping itself rather than from a parallel list — iterate every variant the mapping can produce and assert each either carries an index or appears in a small, explicitly-reasoned exception set. Then adding a type without an index is a test failure that names the type, instead of silence.
If that is awkward, the cheaper version is an exhaustive
matchover the schema-type enum inside the test, so a new variant fails to compile until someone decides which side it belongs on. That is the same shape as themember_set_is_pinnedguard added in #264 — make the omission a build error rather than a passing test.Provenance
Found by an adversarial mutation campaign against merged master
e99d2b5(9 mutations, 8 killed, this one survived). The campaign was reviewing PR #286 and reported after it merged; this is the one finding that is live rather than retrospective. Not introduced by #286 in the sense that no field is currently mis-mapped — the gap is in what the guard can see.