Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
fix: Include vector attributes in Field equality
Re-enables the vector_index and vector_search_metric comparisons in
Field.__eq__, which have been commented out since #4855. Addresses item 5 of
#6907.

They were disabled deliberately, for backwards compatibility: an unset metric
deserializes as "" while the constructor default is None, so a Field never
compared equal to its own deserialized copy and every field lacking an explicit
metric would have shown as changed on every feast plan.

Fixing that asymmetry removes the reason for the exclusion. The normalization
goes in __init__ rather than from_proto, because from_proto is not the only
deserialization path: OnDemandFeatureView._parse_features_from_proto builds a
Field straight from a proto message. Normalizing only from_proto left the two
paths disagreeing, which surfaced as a projection for an aliased on-demand
feature view failing to apply, since the comparison is also what
BaseFeatureView.with_projection uses to check a projection's fields.

With both attributes compared, a change to vector_search_metric from COSINE to
L2, or flipping vector_index, is now visible to feast plan and apply. Previously
such a change altered retrieval semantics while producing no diff.

Verified no regressions by diffing failure names across the full unit suite
before and after: 41 pre-existing failures on both sides, none introduced.

Signed-off-by: hao-xu5 <hxu44@apple.com>
  • Loading branch information
haoxu0 committed Oct 3, 2026
commit 470e64045abbb60318dcedb0adc012b3e7c6e359
12 changes: 9 additions & 3 deletions sdk/python/feast/field.py
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,13 @@ def __init__(
self.tags = tags or {}
self.vector_index = vector_index
self.vector_length = vector_length
self.vector_search_metric = vector_search_metric
# The proto default for an unset metric is "", while the constructor default
# is None. Normalizing here rather than in from_proto covers every
# construction path, including callers that build a Field straight from a
# proto message without going through from_proto. Without this, a Field and
# its own deserialized copy compare unequal, which is why the vector
# comparisons in __eq__ were previously disabled.
self.vector_search_metric = vector_search_metric or None

def __eq__(self, other):
if type(self) != type(other):
Expand All @@ -89,8 +95,8 @@ def __eq__(self, other):
or self.description != other.description
or self.tags != other.tags
or self.vector_length != other.vector_length
# or self.vector_index != other.vector_index
# or self.vector_search_metric != other.vector_search_metric
or self.vector_index != other.vector_index
or self.vector_search_metric != other.vector_search_metric
):
return False
return True
Expand Down
95 changes: 94 additions & 1 deletion sdk/python/tests/unit/test_eq_cross_type.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@
NamespaceBasedPolicy,
RoleBasedPolicy,
)
from feast.types import Int64
from feast.types import Array, Float32, Int64
from feast.value_type import ValueType


Expand Down Expand Up @@ -120,3 +120,96 @@ def test_spark_source_vs_file_source_eq():
assert (file == spark) is False
assert (spark == object()) is False
assert (spark == SparkSource(name="src", table="t", timestamp_field="ts")) is True


def test_field_eq_includes_vector_attributes():
"""vector_index and vector_search_metric participate in Field equality.

They were previously excluded because an unset metric deserialized as "" while
the constructor default was None, so a Field never compared equal to its own
deserialized copy. With that normalized, a change to either attribute is a real
change and must be visible to feast plan / apply.
"""
base = Field(
name="embedding",
dtype=Array(Float32),
vector_index=True,
vector_length=4,
vector_search_metric="COSINE",
)

assert base == Field(
name="embedding",
dtype=Array(Float32),
vector_index=True,
vector_length=4,
vector_search_metric="COSINE",
)

# Changing the metric alters retrieval semantics and must not compare equal.
assert (
base
== Field(
name="embedding",
dtype=Array(Float32),
vector_index=True,
vector_length=4,
vector_search_metric="L2",
)
) is False

# Turning the index off must not compare equal either.
assert (
base
== Field(
name="embedding",
dtype=Array(Float32),
vector_index=False,
vector_length=4,
vector_search_metric="COSINE",
)
) is False


@pytest.mark.parametrize(
"field",
[
Field(name="plain", dtype=Int64),
Field(
name="embedding", dtype=Array(Float32), vector_index=True, vector_length=4
),
Field(
name="embedding",
dtype=Array(Float32),
vector_index=True,
vector_length=4,
vector_search_metric="COSINE",
),
],
ids=["plain", "vector_without_metric", "vector_with_metric"],
)
def test_field_survives_a_proto_round_trip(field):
"""A Field must compare equal to its own deserialized copy.

This is the backwards-compatibility property that previously forced the vector
comparisons out of __eq__: without it, every field lacking an explicit metric
would show as changed on every feast plan.
"""
assert Field.from_proto(field.to_proto()) == field


def test_field_normalizes_an_empty_metric_to_none():
"""An empty metric means unset, whichever construction path supplied it.

Normalizing in __init__ rather than from_proto matters because callers such as
OnDemandFeatureView._parse_features_from_proto build a Field straight from a
proto message without going through from_proto.
"""
assert (
Field(name="x", dtype=Int64, vector_search_metric="").vector_search_metric
is None
)
assert Field(name="x", dtype=Int64).vector_search_metric is None
assert Field(name="x", dtype=Int64, vector_search_metric="") == Field(
name="x", dtype=Int64
)
Loading