Repository navigation
Commit 29af8c0
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
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3619 | 3619 | | |
3620 | 3620 | | |
3621 | 3621 | | |
| 3622 | + | |
| 3623 | + | |
| 3624 | + | |
3622 | 3625 | | |
3623 | 3626 | | |
3624 | 3627 | | |
3625 | 3628 | | |
3626 | 3629 | | |
3627 | 3630 | | |
3628 | 3631 | | |
3629 | | - | |
3630 | | - | |
3631 | | - | |
3632 | | - | |
3633 | | - | |
3634 | | - | |
3635 | | - | |
3636 | | - | |
3637 | | - | |
3638 | | - | |
3639 | | - | |
3640 | | - | |
3641 | | - | |
3642 | | - | |
| 3632 | + | |
| 3633 | + | |
| 3634 | + | |
| 3635 | + | |
| 3636 | + | |
| 3637 | + | |
| 3638 | + | |
| 3639 | + | |
| 3640 | + | |
| 3641 | + | |
| 3642 | + | |
| 3643 | + | |
| 3644 | + | |
| 3645 | + | |
| 3646 | + | |
| 3647 | + | |
| 3648 | + | |
| 3649 | + | |
| 3650 | + | |
| 3651 | + | |
| 3652 | + | |
| 3653 | + | |
| 3654 | + | |
| 3655 | + | |
| 3656 | + | |
| 3657 | + | |
| 3658 | + | |
| 3659 | + | |
| 3660 | + | |
| 3661 | + | |
| 3662 | + | |
| 3663 | + | |
| 3664 | + | |
| 3665 | + | |
| 3666 | + | |
| 3667 | + | |
| 3668 | + | |
| 3669 | + | |
| 3670 | + | |
| 3671 | + | |
| 3672 | + | |
| 3673 | + | |
| 3674 | + | |
| 3675 | + | |
| 3676 | + | |
3643 | 3677 | | |
3644 | 3678 | | |
3645 | 3679 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
548 | 548 | | |
549 | 549 | | |
550 | 550 | | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
551 | 554 | | |
552 | 555 | | |
553 | 556 | | |
| |||
2015 | 2018 | | |
2016 | 2019 | | |
2017 | 2020 | | |
| 2021 | + | |
| 2022 | + | |
| 2023 | + | |
| 2024 | + | |
| 2025 | + | |
| 2026 | + | |
| 2027 | + | |
| 2028 | + | |
| 2029 | + | |
| 2030 | + | |
| 2031 | + | |
| 2032 | + | |
| 2033 | + | |
| 2034 | + | |
| 2035 | + | |
| 2036 | + | |
| 2037 | + | |
| 2038 | + | |
| 2039 | + | |
| 2040 | + | |
| 2041 | + | |
| 2042 | + | |
| 2043 | + | |
| 2044 | + | |
| 2045 | + | |
| 2046 | + | |
| 2047 | + | |
| 2048 | + | |
| 2049 | + | |
| 2050 | + | |
| 2051 | + | |
| 2052 | + | |
| 2053 | + | |
| 2054 | + | |
| 2055 | + | |
| 2056 | + | |
| 2057 | + | |
| 2058 | + | |
| 2059 | + | |
| 2060 | + | |
| 2061 | + | |
| 2062 | + | |
| 2063 | + | |
| 2064 | + | |
| 2065 | + | |
| 2066 | + | |
| 2067 | + | |
| 2068 | + | |
| 2069 | + | |
| 2070 | + | |
| 2071 | + | |
| 2072 | + | |
| 2073 | + | |
| 2074 | + | |
| 2075 | + | |
| 2076 | + | |
| 2077 | + | |
| 2078 | + | |
| 2079 | + | |
| 2080 | + | |
| 2081 | + | |
| 2082 | + | |
| 2083 | + | |
2018 | 2084 | | |
2019 | 2085 | | |
2020 | 2086 | | |
| |||
0 commit comments