fix: add new validator for cosine metric

[iaojnh]

Greptile Summary

This PR adds a new schema validation check that rejects COSINE metric when the field's data type is not VECTOR_FP32 or VECTOR_FP16, preventing configurations such as VECTOR_INT8 + COSINE metric from being accepted. The fix correctly addresses three of the four issues raised in the previous review cycle:

• Allowlist approach adopted: The original guard was a positive equality check against a single type (data_type_ == VECTOR_INT8), which would silently pass any other non-FP type. The new check inverts this — rejecting anything that is not VECTOR_FP16 and not VECTOR_FP32 — making it correctly forward-proof.
• Redundant quantize condition removed: The previous version included an intermediate quantize_type == UNDEFINED sub-condition that was dead code for INT8 data; the new code omits it entirely.
• Error message improved: The updated message includes the field name (field[...]) and the actual offending data type via DataTypeCodeBook::AsString(data_type_), consistent with the style of other error messages in the function.

The check is placed after the sparse/dense and index-type guards, which means sparse vectors cannot reach it (they are already rejected if their metric is not IP), and the code path is logically sound. The validation aligns with the existing IVF+IP guard pattern, though the COSINE check is intentionally broader (index-type-agnostic) since the FP32/FP16 requirement for COSINE is not IVF-specific.

Confidence Score: 4/5

• The PR is safe to merge; it closes a real validation gap and the new guard is logically sound with no new regressions introduced.
• The new allowlist check correctly rejects all non-FP data types for COSINE metric, addresses the previously flagged issues, and the error message is informative. The score is not 5 because one previously-identified gap (COSINE metric with FP32/FP16 data types that are quantized down to INT8/INT4 at index time) was raised in an earlier review round and remains unresolved in this iteration.
• No files require special attention beyond the previously-discussed quantization gap in src/db/index/common/schema.cc.

Important Files Changed

Filename

Overview

src/db/index/common/schema.cc

Adds a COSINE metric validator inside FieldSchema::validate() using a correct allowlist approach (data_type must be VECTOR_FP32 or VECTOR_FP16); error message is informative and includes field name and actual data type. Addresses three of the four previously flagged issues. The one still-open concern (FP32/FP16 data with INT8/INT4 quantization used alongside COSINE metric) was raised in a previous review thread and remains unaddressed.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[FieldSchema::validate] --> B{is_vector_field?}
    B -- No --> C{index_params_ && is_vector_index_type?}
    C -- Yes --> D[Error: scalar field has vector index]
    C -- No --> E[OK]
    B -- Yes --> F{is_sparse?}
    F -- Yes --> G{data_type in support_sparse_vector_type?}
    G -- No --> H[Error: unsupported sparse data type]
    G -- Yes --> I{index_params_?}
    F -- No --> J{data_type in support_dense_vector_type?}
    J -- No --> K[Error: unsupported dense data type]
    J -- Yes --> I
    I -- No --> L[OK]
    I -- Yes --> M{is_sparse?}
    M -- Yes --> N{index in support_sparse_vector_index?}
    N -- No --> O[Error: unsupported sparse index]
    N -- Yes --> P{metric == IP?}
    P -- No --> Q[Error: sparse only supports IP]
    P -- Yes --> R
    M -- No --> S{index in support_dense_vector_index?}
    S -- No --> T[Error: unsupported dense index]
    S -- Yes --> R
    R{quantize_type != UNDEFINED?} -- Yes --> U{data_type in quantize_type_map?}
    U -- No --> V[Error: data type does not support quantize]
    U -- Yes --> W{quantize_type in allowed set?}
    W -- No --> X[Error: unsupported quantize type]
    W -- Yes --> Y
    R -- No --> Y
    Y{IVF index AND IP metric?} -- Yes --> Z{data_type FP32 or FP16?}
    Z -- No --> AA[Error: IVF+IP only supports FP32/FP16]
    Z -- Yes --> AB
    Y -- No --> AB
    AB{metric == COSINE?} -- Yes --> AC{data_type FP32 or FP16?}
    AC -- No --> AD[NEW: Error: cosine only supports FP32/FP16, field name + actual type]
    AC -- Yes --> AE[OK]
    AB -- No --> AE

Loading

_{Last reviewed commit: 5e01a0c}

[greptile-apps]

Greptile Summary

This PR adds a validator inside FieldSchema::validate() that rejects any COSINE-metric field whose storage data_type_ is not VECTOR_FP32 or VECTOR_FP16. The implementation correctly uses an inverted condition (!= VECTOR_FP16 && != VECTOR_FP32), which future-proofs the check against any new non-FP dense types. The error message includes the field name and actual data type, consistent with other validation messages in the function. The validator is placed appropriately within the shared if (index_params_) block and applies to both sparse and dense paths; sparse vectors with COSINE metric are already rejected earlier (lines 109–116). One known limitation remains as a separate follow-up: VECTOR_FP32 or VECTOR_FP16 combined with an INT8/INT4 quantize_type and COSINE metric will not be caught by this check, as the validator only examines the base data type, not the quantization type. This is a pre-existing architectural consideration rather than a regression introduced by this PR.

Confidence Score: 4/5

• This PR is safe to merge; it adds a net-positive validation guard for COSINE metrics with no regressions.
• The COSINE metric validator is correctly implemented with an inverted condition that is future-proof against additional non-FP dense types. The error message includes field name and data type, consistent with the codebase style. The validator is appropriately placed and handles the primary case (native INT8 data type with COSINE). The known limitation regarding quantized FP32/FP16 vectors combined with COSINE is a pre-existing architectural gap, not a regression introduced by this PR, and is appropriate for a follow-up effort.
• No files require special attention; the single changed file is well-structured and the new guard follows established validation patterns.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[FieldSchema::validate] --> B{is_vector_field?}
    B -- No --> Z[Scalar index check]
    B -- Yes --> C{is_sparse?}
    C -- Yes --> D{data_type in\nsupport_sparse_vector_type?}
    D -- No --> ERR1[InvalidArgument:\nunsupported sparse data type]
    D -- Yes --> E{metric == IP?}
    E -- No --> ERR2[InvalidArgument:\nsparse only supports IP metric]
    E -- Yes --> F{index_params set?}
    C -- No --> G{data_type in\nsupport_dense_vector_type?}
    G -- No --> ERR3[InvalidArgument:\nunsupported dense data type]
    G -- Yes --> H{index_type in\nsupport_dense_vector_index?}
    H -- No --> ERR4[InvalidArgument:\nunsupported dense index type]
    H -- Yes --> F
    F -- No --> OK[OK]
    F -- Yes --> I{quantize_type != UNDEFINED?}
    I -- Yes --> J{data_type in\nquantize_type_map?}
    J -- No --> ERR5[InvalidArgument:\ndata type does not support quantize]
    J -- Yes --> K{quantize_type in\nallowed set?}
    K -- No --> ERR6[InvalidArgument:\nunsupported quantize type]
    K -- Yes --> L{IVF && IP?}
    I -- No --> L
    L -- Yes --> M{data_type is\nFP16 or FP32?}
    M -- No --> ERR7[InvalidArgument:\nIVF+IP requires FP32/FP16]
    M -- Yes --> N
    L -- No --> N{metric == COSINE?\n🆕 NEW CHECK}
    N -- Yes --> O{data_type is\nFP16 or FP32?}
    O -- No --> ERR8[InvalidArgument:\nCOSINE requires FP32/FP16\nfield name + actual data type]
    O -- Yes --> OK
    N -- No --> OK

Loading

_{Last reviewed commit: 5e01a0c}

[iaojnh]

@greptile

[iaojnh]

@greptile

[iaojnh]

@greptile

[feihongxu0824]

Why is cosine + int8 not supported at this moment?

[iaojnh]

Why is cosine + int8 not supported at this moment?

Currently, corresponding methods are lacking and still need to be developed.

[Cuiyus]

@greptile