Skip to content

Commit 29af8c0

Browse files
authored
fix: Make vector length validation reachable, schema-driven and vectorized (#6909)
* fix: Make vector length validation reachable, schema-driven and vectorized Addresses three of the five items in #6907. Validation was reachable only from the online-write path. _validate_vector_features had a single call site inside _get_feature_view_and_df_for_online_write, so materialize, materialize_incremental and get_historical_features never ran it. Embedding workloads are overwhelmingly batch, so the path carrying vectors at volume was the unvalidated one. Every compute engine (local, spark, ray, flink, kubernetes, aws_lambda) and the passthrough provider funnel through utils._convert_arrow_to_proto, so a single Arrow-level check placed in _convert_arrow_fv_to_proto covers all of them. _validate_vector_field_lengths is O(1) for fixed-size lists and one vectorized pass for variable-size lists, so it is cheap enough to leave on. Null rows are tolerated; a non-list column with a declared vector_length is an error. Note the snowflake compute engine builds its rows without going through _convert_arrow_to_proto, so it is not covered by this change. The check assumed the vector was the first feature. feature_view.features[0].vector_index meant that declaring any field before the vector field silently disabled validation. Both validators now resolve the field via _get_feature_view_vector_field_metadata, which scans the schema and is already used elsewhere in feature_store.py. iterrows() did not scale. Replaced with a single pass over the column, which measured ~60x faster on 200k rows (1.51s to 0.025s) while keeping the row index and the two distinct failure messages. Not included, since #6907 raises them as behaviour decisions rather than fixes: requiring vector_length when vector_index=True (item 3), and whether Field.__eq__ should compare vector_index and vector_search_metric (item 5). Adds sdk/python/tests/unit/test_vector_length_validation.py with 16 tests covering both validators, including regression tests for the vector field not being first. Signed-off-by: hao-xu5 <[email protected]> * fix: Address review feedback on vector length validation Three findings from review of #6909. Tolerate genuine nulls on the DataFrame path (@ntkathole). lengths.isna() was true both for values that were not sequences and for rows that were genuinely null, so a null vector was reported as "not a sequence". Nulls are now skipped, which also makes this path consistent with the Arrow path, which already tolerated null rows. Report the first offending row across both failure modes (Copilot). Checking the not-a-sequence mask fully before the length mask meant a non-sequence in a later row was reported ahead of a wrong length in an earlier one. The masks are now unioned and the first offending position selected before choosing the message, which restores the row ordering the original iterrows loop had. Value lookup is positional so a DataFrame with a duplicate index reports a scalar type rather than a Series. Validate the on-demand branch too (Copilot). _convert_arrow_to_proto dispatches to _convert_arrow_odfv_to_proto or _convert_arrow_fv_to_proto, and the check sat in the regular branch only, so a stored ODFV declaring vector_length had its transformed output unchecked. The call moved up into the dispatcher, ahead of the branch, so both paths are covered. Each finding has a test that fails against the previous commit and passes here. Signed-off-by: hao-xu5 <[email protected]> --------- Signed-off-by: hao-xu5 <[email protected]>
1 parent 374f1d7 commit 29af8c0

3 files changed

Lines changed: 366 additions & 14 deletions

File tree

‎sdk/python/feast/feature_store.py‎

Lines changed: 48 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3619,27 +3619,61 @@ def _validate_vector_features(self, feature_view, df: pd.DataFrame) -> None:
36193619
"""
36203620
Validates vector features in the DataFrame against the feature view specifications.
36213621
3622+
Resolves the vector field from the feature view schema rather than assuming it is
3623+
the first feature, and checks lengths in a single pass over the column.
3624+
36223625
Args:
36233626
feature_view: The feature view containing vector feature specifications
36243627
df: The DataFrame to validate
36253628
36263629
Raises:
36273630
ValueError: If vector dimension constraints are violated
36283631
"""
3629-
if feature_view.features and feature_view.features[0].vector_index:
3630-
fv_vector_feature_name = feature_view.features[0].name
3631-
if feature_view.features[0].vector_length != 0:
3632-
for i, row in df.iterrows():
3633-
vector = row[fv_vector_feature_name]
3634-
if not hasattr(vector, "__len__"):
3635-
raise ValueError(
3636-
f"Row {i}: Vector feature '{fv_vector_feature_name}' is not a sequence. Got: {type(vector)}"
3637-
)
3638-
if len(vector) != feature_view.features[0].vector_length:
3639-
raise ValueError(
3640-
f"Row {i}: Vector length {len(vector)} does not match expected {feature_view.features[0].vector_length} "
3641-
f"for feature '{fv_vector_feature_name}' in feature view '{feature_view.name}'."
3642-
)
3632+
vector_field = _get_feature_view_vector_field_metadata(feature_view)
3633+
if vector_field is None or not vector_field.vector_length:
3634+
return
3635+
3636+
name = vector_field.name
3637+
if name not in df.columns:
3638+
return
3639+
3640+
expected = vector_field.vector_length
3641+
column = df[name]
3642+
3643+
# Null vectors carry no length to compare. Skipping them keeps this
3644+
# consistent with the Arrow path, which also tolerates null rows, and
3645+
# stops a genuine null being reported as "not a sequence".
3646+
is_null = column.isna()
3647+
3648+
# Single pass over the column. na_action leaves nulls as NaN, so a NaN
3649+
# length that is not null means the value was not a sequence at all.
3650+
lengths = column.map(
3651+
lambda v: len(v) if hasattr(v, "__len__") else None, na_action="ignore"
3652+
)
3653+
3654+
not_a_sequence = lengths.isna() & ~is_null
3655+
mismatched = lengths.notna() & (lengths != expected)
3656+
offending = not_a_sequence | mismatched
3657+
if not offending.any():
3658+
return
3659+
3660+
# Report the first offending row across both failure modes, by position,
3661+
# so the message is right regardless of which failure comes first and
3662+
# correct even when the index has duplicates.
3663+
position = int(offending.to_numpy().argmax())
3664+
label = column.index[position]
3665+
3666+
if bool(not_a_sequence.to_numpy()[position]):
3667+
raise ValueError(
3668+
f"Row {label}: Vector feature '{name}' is not a sequence. "
3669+
f"Got: {type(column.iloc[position])}"
3670+
)
3671+
3672+
raise ValueError(
3673+
f"Row {label}: Vector length {int(lengths.iloc[position])} does not match "
3674+
f"expected {expected} for feature '{name}' in feature view "
3675+
f"'{feature_view.name}'."
3676+
)
36433677

36443678
def _get_feature_view_and_df_for_online_write(
36453679
self,

‎sdk/python/feast/utils.py‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -548,6 +548,9 @@ def _convert_arrow_to_proto(
548548
) -> List[Tuple[EntityKeyProto, Dict[str, ValueProto], datetime, Optional[datetime]]]:
549549
# This is a workaround for isinstance(feature_view, OnDemandFeatureView), which triggers a circular import
550550
# Check for source_request_sources or source_feature_view_projections attributes to identify ODFVs
551+
# Validate before branching so both regular and on-demand feature views are covered.
552+
_validate_vector_field_lengths(table, feature_view)
553+
551554
if (
552555
getattr(feature_view, "source_request_sources", None) is not None
553556
or getattr(feature_view, "source_feature_view_projections", None) is not None
@@ -2015,6 +2018,69 @@ def _get_feature_view_vector_field_metadata(
20152018
return vector_fields[0]
20162019

20172020

2021+
def _validate_vector_field_lengths(
2022+
table: Union[pyarrow.Table, pyarrow.RecordBatch],
2023+
feature_view,
2024+
) -> None:
2025+
"""Check an Arrow table's vector column against the declared ``vector_length``.
2026+
2027+
Called on the materialization path, where every compute engine funnels through
2028+
``_convert_arrow_to_proto``. The check is O(1) for fixed-size lists and a single
2029+
vectorized pass for variable-size lists, so it is safe to leave on.
2030+
2031+
A declared ``vector_length`` is a contract: a mismatch is an error, never a
2032+
silent truncation or a silently different response shape.
2033+
2034+
Args:
2035+
table: The Arrow table or record batch about to be converted.
2036+
feature_view: The feature view whose schema declares the vector field.
2037+
2038+
Raises:
2039+
ValueError: If the vector column's width disagrees with ``vector_length``.
2040+
"""
2041+
vector_field = _get_feature_view_vector_field_metadata(feature_view)
2042+
if vector_field is None or not vector_field.vector_length:
2043+
return
2044+
2045+
name = vector_field.name
2046+
if name not in table.schema.names:
2047+
return
2048+
2049+
expected = vector_field.vector_length
2050+
column = table.column(name)
2051+
column_type = column.type
2052+
2053+
def _fail(actual, row: Optional[int] = None) -> None:
2054+
where = f"Row {row}: " if row is not None else ""
2055+
raise ValueError(
2056+
f"{where}Vector length {actual} does not match expected {expected} "
2057+
f"for feature '{name}' in feature view '{feature_view.name}'."
2058+
)
2059+
2060+
if pyarrow.types.is_fixed_size_list(column_type):
2061+
if column_type.list_size != expected:
2062+
_fail(column_type.list_size)
2063+
return
2064+
2065+
if pyarrow.types.is_list(column_type) or pyarrow.types.is_large_list(column_type):
2066+
lengths = pyarrow.compute.list_value_length(column)
2067+
# Nulls have no length to compare; only non-null rows carry the contract.
2068+
mismatched = pyarrow.compute.and_kleene(
2069+
pyarrow.compute.is_valid(lengths),
2070+
pyarrow.compute.not_equal(lengths, expected),
2071+
)
2072+
if pyarrow.compute.any(mismatched).as_py():
2073+
row = pyarrow.compute.index(mismatched, True).as_py()
2074+
_fail(lengths[row].as_py(), row=row)
2075+
return
2076+
2077+
# Any other type cannot hold a vector of the declared width.
2078+
raise ValueError(
2079+
f"Vector feature '{name}' in feature view '{feature_view.name}' declares "
2080+
f"vector_length={expected} but has non-list Arrow type {column_type}."
2081+
)
2082+
2083+
20182084
def _distance_to_score(distance: float, metric: Optional[str] = None) -> float:
20192085
"""Convert a raw distance value into a higher-is-better relevance score.
20202086

0 commit comments

Comments
 (0)