Skip to content

Commit 0056565

Browse files
haoxu0ntkathole
haoxu0
authored and
ntkathole
committed
fix: Infer vector width when vector_length is undeclared
Addresses item 3 of #6907, which asked for vector_length to be required when vector_index=True. That specific change is not viable, so this takes a non-breaking route to the same goal. Requiring it would break roughly 29 call sites that declare vector_index=True without vector_length, including the bundled feast init -t rag and -t ray_rag templates, several entries under examples/, and four online stores that deliberately fall back to 512. The onboarding path would stop working. The underlying problem is still real: vector_length defaults to 0 and 0 meant skip, so the common case was unvalidated. Instead of skipping, the width of the first non-null row now becomes the contract for the rest of the table. An approximate nearest neighbour index requires a fixed width, so a ragged vector column is a defect whether or not anyone declared a length, and inferring is strictly better than skipping. A declared vector_length still wins over inference, so nothing about the declared case changes. Fixed-size list columns are uniform by construction, so an undeclared one needs no check at all. Null rows carry no width and are neither used for inference nor compared. The gate moves from vector_length to vector_index, since vector_index is what says the field is a vector. Verified no regressions by diffing failure names across the full unit suite before and after: 41 pre-existing failures on both sides, none introduced, which also confirms none of the undeclared sites produce ragged columns. Signed-off-by: hao-xu5 <[email protected]>
1 parent 1196e22 commit 0056565

3 files changed

Lines changed: 139 additions & 20 deletions

File tree

‎sdk/python/feast/feature_store.py‎

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3630,15 +3630,15 @@ def _validate_vector_features(self, feature_view, df: pd.DataFrame) -> None:
36303630
ValueError: If vector dimension constraints are violated
36313631
"""
36323632
vector_field = _get_feature_view_vector_field_metadata(feature_view)
3633-
if vector_field is None or not vector_field.vector_length:
3633+
if vector_field is None or not vector_field.vector_index:
36343634
return
36353635

36363636
name = vector_field.name
36373637
if name not in df.columns:
36383638
return
36393639

3640-
expected = vector_field.vector_length
36413640
column = df[name]
3641+
declared = vector_field.vector_length or None
36423642

36433643
# Null vectors carry no length to compare. Skipping them keeps this
36443644
# consistent with the Arrow path, which also tolerates null rows, and
@@ -3652,6 +3652,21 @@ def _validate_vector_features(self, feature_view, df: pd.DataFrame) -> None:
36523652
)
36533653

36543654
not_a_sequence = lengths.isna() & ~is_null
3655+
3656+
expected = declared
3657+
if expected is None:
3658+
# No declared width, so take the contract from the first usable row.
3659+
# An ANN index needs a fixed width, so a ragged column is a defect
3660+
# regardless of whether anyone declared vector_length.
3661+
usable = lengths.notna()
3662+
if not usable.any():
3663+
# Nothing to infer from, so only the type check below applies.
3664+
if not not_a_sequence.any():
3665+
return
3666+
expected = -1
3667+
else:
3668+
expected = int(lengths[usable].iloc[0])
3669+
36553670
mismatched = lengths.notna() & (lengths != expected)
36563671
offending = not_a_sequence | mismatched
36573672
if not offending.any():
@@ -3669,10 +3684,14 @@ def _validate_vector_features(self, feature_view, df: pd.DataFrame) -> None:
36693684
f"Got: {type(column.iloc[position])}"
36703685
)
36713686

3687+
source = (
3688+
f"expected {expected}"
3689+
if declared
3690+
else f"expected {expected}, inferred from the first row"
3691+
)
36723692
raise ValueError(
36733693
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}'."
3694+
f"{source} for feature '{name}' in feature view '{feature_view.name}'."
36763695
)
36773696

36783697
def _get_feature_view_and_df_for_online_write(

‎sdk/python/feast/utils.py‎

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -2027,62 +2027,86 @@ def _validate_vector_field_lengths(
20272027
table: Union[pyarrow.Table, pyarrow.RecordBatch],
20282028
feature_view,
20292029
) -> None:
2030-
"""Check an Arrow table's vector column against the declared ``vector_length``.
2030+
"""Check an Arrow table's vector column for a consistent width.
20312031
20322032
Called on the materialization path, where every compute engine funnels through
20332033
``_convert_arrow_to_proto``. The check is O(1) for fixed-size lists and a single
20342034
vectorized pass for variable-size lists, so it is safe to leave on.
20352035
2036-
A declared ``vector_length`` is a contract: a mismatch is an error, never a
2037-
silent truncation or a silently different response shape.
2036+
When the field declares a ``vector_length``, that is the contract and a mismatch
2037+
is an error, never a silent truncation or a silently different response shape.
2038+
2039+
When it does not, the width of the first non-null row is used as the contract for
2040+
the rest of the table. An approximate nearest neighbour index requires a fixed
2041+
width, so a ragged vector column is a defect either way, and inferring is strictly
2042+
better than skipping. ``vector_length`` is unset by default, including in the
2043+
bundled RAG templates, so skipping would leave the common case unchecked.
20382044
20392045
Args:
20402046
table: The Arrow table or record batch about to be converted.
20412047
feature_view: The feature view whose schema declares the vector field.
20422048
20432049
Raises:
2044-
ValueError: If the vector column's width disagrees with ``vector_length``.
2050+
ValueError: If the vector column's width is inconsistent, or disagrees with
2051+
a declared ``vector_length``.
20452052
"""
20462053
vector_field = _get_feature_view_vector_field_metadata(feature_view)
2047-
if vector_field is None or not vector_field.vector_length:
2054+
if vector_field is None or not vector_field.vector_index:
20482055
return
20492056

20502057
name = vector_field.name
20512058
if name not in table.schema.names:
20522059
return
20532060

2054-
expected = vector_field.vector_length
20552061
column = table.column(name)
20562062
column_type = column.type
2063+
declared = vector_field.vector_length or None
20572064

2058-
def _fail(actual, row: Optional[int] = None) -> None:
2065+
def _fail(actual, expected: int, row: Optional[int] = None) -> None:
20592066
where = f"Row {row}: " if row is not None else ""
2067+
source = (
2068+
f"expected {expected}"
2069+
if declared
2070+
else f"expected {expected}, inferred from the first row"
2071+
)
20602072
raise ValueError(
2061-
f"{where}Vector length {actual} does not match expected {expected} "
2073+
f"{where}Vector length {actual} does not match {source} "
20622074
f"for feature '{name}' in feature view '{feature_view.name}'."
20632075
)
20642076

20652077
if pyarrow.types.is_fixed_size_list(column_type):
2066-
if column_type.list_size != expected:
2067-
_fail(column_type.list_size)
2078+
# The width is a property of the type, so an undeclared column is uniform
2079+
# by construction and there is nothing left to check.
2080+
if declared and column_type.list_size != declared:
2081+
_fail(column_type.list_size, declared)
20682082
return
20692083

20702084
if pyarrow.types.is_list(column_type) or pyarrow.types.is_large_list(column_type):
20712085
lengths = pyarrow.compute.list_value_length(column)
2086+
valid = pyarrow.compute.is_valid(lengths)
2087+
2088+
expected = declared
2089+
if expected is None:
2090+
# Nulls carry no width, so take the contract from the first row that does.
2091+
first = pyarrow.compute.index(valid, True).as_py()
2092+
if first == -1:
2093+
return
2094+
expected = lengths[first].as_py()
2095+
20722096
# Nulls have no length to compare; only non-null rows carry the contract.
20732097
mismatched = pyarrow.compute.and_kleene(
2074-
pyarrow.compute.is_valid(lengths),
2075-
pyarrow.compute.not_equal(lengths, expected),
2098+
valid, pyarrow.compute.not_equal(lengths, expected)
20762099
)
20772100
if pyarrow.compute.any(mismatched).as_py():
20782101
row = pyarrow.compute.index(mismatched, True).as_py()
2079-
_fail(lengths[row].as_py(), row=row)
2102+
_fail(lengths[row].as_py(), expected, row=row)
20802103
return
20812104

2082-
# Any other type cannot hold a vector of the declared width.
2105+
# Any other type cannot hold a vector.
2106+
declared_note = f" declares vector_length={declared} but" if declared else ""
20832107
raise ValueError(
2084-
f"Vector feature '{name}' in feature view '{feature_view.name}' declares "
2085-
f"vector_length={expected} but has non-list Arrow type {column_type}."
2108+
f"Vector feature '{name}' in feature view '{feature_view.name}'{declared_note} "
2109+
f"has non-list Arrow type {column_type}."
20862110
)
20872111

20882112

‎sdk/python/tests/unit/test_vector_length_validation.py‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,3 +250,79 @@ def odfv(inputs):
250250
)
251251
with pytest.raises(ValueError, match="does not match expected 4"):
252252
_convert_arrow_to_proto(table, odfv, {})
253+
254+
255+
class TestUndeclaredVectorLength:
256+
"""When vector_length is unset, the width is inferred rather than unchecked.
257+
258+
vector_length defaults to 0, including in the bundled RAG templates and several
259+
examples, so skipping validation left the common case unprotected. An ANN index
260+
requires a fixed width, so a ragged column is a defect either way.
261+
"""
262+
263+
def test_arrow_ragged_column_is_rejected_without_a_declaration(self):
264+
table = _table(_variable_list([4, 4, 7, 4]))
265+
with pytest.raises(ValueError, match="inferred from the first row"):
266+
_validate_vector_field_lengths(table, _feature_view(vector_length=0))
267+
268+
def test_arrow_uniform_column_passes_without_a_declaration(self):
269+
table = _table(_variable_list([5, 5, 5]))
270+
_validate_vector_field_lengths(table, _feature_view(vector_length=0))
271+
272+
def test_arrow_fixed_size_list_is_uniform_by_construction(self):
273+
"""A fixed-size list cannot be ragged, so any width is acceptable."""
274+
_validate_vector_field_lengths(
275+
_table(_fixed_size_list(99)), _feature_view(vector_length=0)
276+
)
277+
278+
def test_arrow_declaration_still_wins_over_inference(self):
279+
table = _table(_variable_list([5, 5, 5]))
280+
with pytest.raises(ValueError, match="expected 4 for feature"):
281+
_validate_vector_field_lengths(table, _feature_view())
282+
283+
def test_arrow_nulls_do_not_become_the_inferred_width(self):
284+
table = _table(_variable_list([None, 6, 6]))
285+
_validate_vector_field_lengths(table, _feature_view(vector_length=0))
286+
287+
def test_arrow_all_null_column_is_a_no_op(self):
288+
table = _table(_variable_list([None, None]))
289+
_validate_vector_field_lengths(table, _feature_view(vector_length=0))
290+
291+
def test_dataframe_ragged_column_is_rejected_without_a_declaration(self):
292+
df = pd.DataFrame(
293+
{"embedding": [[0.0] * 4, [0.0] * 4, [0.0] * 7], "label": list("abc")}
294+
)
295+
with pytest.raises(ValueError, match="inferred from the first row"):
296+
_validate_df(_feature_view(vector_length=0), df)
297+
298+
def test_dataframe_uniform_column_passes_without_a_declaration(self):
299+
df = pd.DataFrame({"embedding": [[0.0] * 9] * 3, "label": list("abc")})
300+
_validate_df(_feature_view(vector_length=0), df)
301+
302+
def test_dataframe_non_sequence_still_caught_without_a_declaration(self):
303+
df = pd.DataFrame({"embedding": [[0.0] * 4, 3.14], "label": list("ab")})
304+
with pytest.raises(ValueError, match="is not a sequence"):
305+
_validate_df(_feature_view(vector_length=0), df)
306+
307+
def test_dataframe_all_null_column_is_a_no_op(self):
308+
df = pd.DataFrame({"embedding": [None, np.nan], "label": list("ab")})
309+
_validate_df(_feature_view(vector_length=0), df)
310+
311+
def test_non_vector_field_is_untouched(self):
312+
"""vector_index=False means the field is not a vector at all."""
313+
fv = FeatureView(
314+
name="fv",
315+
entities=[
316+
Entity(
317+
name="item_id", join_keys=["item_id"], value_type=ValueType.INT64
318+
)
319+
],
320+
schema=[
321+
Field(name="embedding", dtype=Array(Float32)),
322+
Field(name="label", dtype=String),
323+
],
324+
source=FileSource(
325+
path="/tmp/unused.parquet", timestamp_field="event_timestamp"
326+
),
327+
)
328+
_validate_vector_field_lengths(_table(_variable_list([4, 9, 2])), fv)

0 commit comments

Comments
 (0)