FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Vector length validation is unreachable from the batch path, skipped unless the vector is the first field, and off by default · Issue #6907 · feast-dev/feast · GitHub

Repository navigation

Vector length validation is unreachable from the batch path, skipped unless the vector is the first field, and off by default #6907

Description

Summary

vector_length is declared on Field and there is a validator for it, but the validator is reachable from only one code path and has three conditions that silently disable it. The net effect is that a feature view can declare vector_length=160, be materialized with 200-dimensional vectors, and Feast will not complain.

All line numbers are against master as of filing.

1. Validation only runs on the online-write path

_validate_vector_features is defined at feature_store.py:3618 and has exactly one call site, feature_store.py:3665, inside _get_feature_view_and_df_for_online_write.

So:

entry point validated?
write_to_online_store (L3677) yes
materialize (L2835) no
materialize_incremental (L2592) no
get_historical_features (L1988) no

Embedding workloads are overwhelmingly batch. The path that actually carries vectors at volume is the unvalidated one.

2. The check assumes the vector is the first feature

feature_store.py:3629:

if feature_view.features and feature_view.features[0].vector_index:

If the vector field is not at index 0, features[0].vector_index is False and the whole check is skipped. Declaring Field(name="id", ...) before Field(name="embedding", vector_index=True, vector_length=160) is enough to disable validation, with no warning.

There is already a helper that does this correctly — utils.py:1959 _get_feature_view_vector_field_metadata(), which scans feature_view.schema for vector_index and raises if there is more than one (L1965). Several online stores already use it. The validator should too.

3. vector_length=0 is the default, and 0 means "skip"

feature_store.py:3631:

if feature_view.features[0].vector_length != 0:

and the default is 0 (field.py:60). So Field(name="embedding", dtype=Array(Float32), vector_index=True) — a perfectly natural declaration — validates nothing. Opt-in-by-accident is the wrong default for a correctness check.

Suggestion: when vector_index=True, require vector_length, or infer it once from the first batch and then enforce it consistently. Either is better than silently doing nothing.

4. df.iterrows() does not scale

feature_store.py:3632 validates with a per-row Python loop. For embedding tables this is the dominant cost and effectively forces users to turn validation off. A vectorized length check over the column (or an Arrow-level check) is equivalent and ~free.

5. Field.__eq__ ignores two of the three vector attributes

field.py:91-93:

or self.vector_length != other.vector_length
# or self.vector_index != other.vector_index
# or self.vector_search_metric != other.vector_search_metric

Only vector_length participates in equality. Changing vector_search_metric from COSINE to L2, or flipping vector_index, is therefore invisible to feast apply / feast plan — a change that alters retrieval semantics produces no diff.

If these are commented out deliberately (e.g. to avoid churn on registries written before the fields existed), that is worth a comment saying so, because as written it reads like an oversight.

Why this matters

Reviewing an unrelated production embedding pipeline recently, I found the same class of bug with a worse outcome: instead of raising, it silently truncated vectors to the declared width (vector.take(dimension)), and the declared width itself fell back to "length of the first row" when absent from config. Nothing downstream verified it, and the resulting recall loss was invisible because no quality metric was attached to the artifact.

Feast gets the hard part right — it raises rather than truncates. The problem is only that the raise is unreachable from the batch path, skipped when the vector is not the first field, and off by default. Those are cheap to fix and they are what makes the guarantee real.

Proposed changes

  1. Call _validate_vector_features from the materialize / historical-retrieval paths, not only online write.
  2. Resolve the vector field via _get_feature_view_vector_field_metadata() instead of features[0].
  3. Require vector_length when vector_index=True, or enforce a consistent inferred value.
  4. Replace iterrows() with a vectorized length check.
  5. Decide Field.__eq__ intent for vector_index / vector_search_metric — include them, or document why not.

Happy to send this as a PR; filing first since (3) and (5) are behaviour decisions rather than clear-cut fixes.

Activity

  1. self-assigned this
    on Sep 30, 2026
  2. haoxu0 commented on Oct 3, 2026

    CollaboratorAuthor

    Status update — all five items now have PRs.

    item what status
    1 validation unreachable from the batch path #6909 merged (29af8c0ab)
    2 features[0] positional assumption #6909 merged
    4 iterrows() did not scale #6909 merged (~60x on 200k rows)
    5 Field.__eq__ ignores vector_index / vector_search_metric #6934
    3 vector_length=0 is the default and means "skip" #6935

    Two corrections to what I originally wrote here, both from digging into why the code was the way it was.

    Item 5 was deliberate, not an oversight. I guessed as much in the original text; 1035cd4ab from #4855 confirms it:

    "Have to remove the equality test for the new fields...for now we're going to ignore them so it is backwards compatible"

    The same commit changed from_proto to getattr(field_proto, 'vector_search_metric', ''), which made an unset metric deserialize as "" while the constructor default is None. With the comparison enabled, every field lacking an explicit metric compared unequal to its own deserialized copy, so every feast plan would report spurious changes. #6934 fixes that asymmetry, which removes the reason for the exclusion — rather than just uncommenting and reintroducing the churn.

    Item 3 as written is not viable. I proposed requiring vector_length when vector_index=True. There are ~29 call sites that do not set it, including feast init -t rag and -t ray_rag, several examples/, and four online stores that fall back to 512. Requiring it would break the onboarding path.

    #6935 takes a non-breaking route to the same goal instead: when vector_length is undeclared, infer the width from the first non-null row and enforce it across the rest of the table, rather than skipping. An ANN index needs a fixed width, so a ragged column is a defect either way, and no existing declaration has to change.

    Happy to close this once #6934 and #6935 land.

  3. haoxu0 commented on Oct 4, 2026

    CollaboratorAuthor

    All five items are now merged.

    item what PR commit
    1 validation unreachable from the batch path #6909 29af8c0ab
    2 features[0] positional assumption #6909 29af8c0ab
    4 iterrows() did not scale (~60x on 200k rows) #6909 29af8c0ab
    5 Field.__eq__ ignored vector_index / vector_search_metric #6934 298c3f6a4
    3 vector_length=0 meant "skip" #6935 005656522

    Two of the five did not land the way this issue originally described them, both because digging into why the code was that way changed the answer:

    Item 5 was deliberate, not an oversight — 1035cd4ab from #4855 disabled the comparisons for backwards compatibility, because an unset metric deserialized as "" against a None constructor default, so every field lacking an explicit metric compared unequal to its own deserialized copy. #6934 fixed that asymmetry in Field.__init__ instead, which removed the reason for the exclusion rather than reintroducing the plan churn.

    Item 3 as proposed was not viable — requiring vector_length when vector_index=True would have broken ~29 call sites including feast init -t rag and -t ray_rag. #6935 instead infers the width from the first non-null row when it is undeclared, which closes the same hole without requiring any existing declaration to change.

    Closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions


    Back | FazBrowse Home | New Git URL