Skip to content

fix: Merge shared ODFV source projections in feature resolution#6622

Open
sanskar-singh-2403 wants to merge 1 commit into
feast-dev:masterfrom
sanskar-singh-2403:fix/odfv-shared-source-projection-merge
Open

fix: Merge shared ODFV source projections in feature resolution#6622
sanskar-singh-2403 wants to merge 1 commit into
feast-dev:masterfrom
sanskar-singh-2403:fix/odfv-shared-source-projection-merge

Conversation

@sanskar-singh-2403

Copy link
Copy Markdown

What this does
Fixes #6621.

When several on demand feature views share a source feature view but select different input columns, _get_feature_views_to_use appended the shared source view once per ODFV, each copy carrying only that ODFV's projection. The membership guard if source_fv not in fvs_to_use never matched, because BaseFeatureView.eq compares projections and the freshly-fetched full-projection view never equals an already-stored narrowed copy.

Downstream, _group_feature_refs keys its view index by projection.name_to_use(), so the duplicate entries collide and the last one wins, the other ODFVs' source projections are silently dropped. During get_historical_features this means the losing ODFV's input columns are never fetched, so transforms receive missing inputs (NaN in pandas mode).

The offline retrieval path is masked on 0.62+ by the ref-injection loop from #6140, but the resolution helper still returns duplicate entries with conflicting projections to its other callers (_get_online_request_context, retrieve_online_documents, retrieve_online_documents_v2).

The fix
In _get_feature_views_to_use, index accumulated source views by projection.name_to_use() and, on a repeat hit, merge the projections (union of features, deduped by name) instead of appending a duplicate. A shared source view now resolves to a single entry carrying the union of the required input columns.

The merge helper reassigns projection.features to a fresh list rather than mutating in place, because the projections in play are shallow copies (copy.copy) that share their features list with the registry object.

Tests
Added TestGetFeatureViewsToUseSharedSource in tests/unit/test_utils.py:

  • test_shared_source_projections_are_merged two ODFVs (odfv_a on src_fv[["a"]], odfv_b on src_fv[["b"]]) resolve to a single src_fv entry with projection ["a", "b"].
  • test_shared_source_ref_order_independent result is the same regardless of ODFV request order.

Both fail on master (assert 2 == 1 the duplicate entries) and pass with this change. ruff check and ruff format are clean.

Related
#6099, #6140

@sanskar-singh-2403
sanskar-singh-2403 requested a review from a team as a code owner July 22, 2026 05:43
When several on demand feature views share a source feature view but
select different input columns, _get_feature_views_to_use appended the
shared source view once per ODFV, each copy carrying only that ODFV's
projection. The membership check 'if source_fv not in fvs_to_use' never
matched because BaseFeatureView.__eq__ compares projections and the
freshly fetched full-projection view never equals a stored narrowed
copy. Downstream, _group_feature_refs keys its view index by
projection.name_to_use(), so the last duplicate won and the other
ODFVs' source projections were silently dropped, producing missing
inputs (NaN in pandas mode) during retrieval.

Index accumulated source views by projection.name_to_use() and merge
projections (union of features, deduped by name) instead of appending a
duplicate, so a shared source view resolves to a single entry carrying
the union of the required input columns.

Fixes feast-dev#6621

Signed-off-by: Sanskar Singh <sanskarsinghty1234@gmail.com>
@sanskar-singh-2403
sanskar-singh-2403 force-pushed the fix/odfv-shared-source-projection-merge branch from 072cf48 to 621c1b1 Compare July 22, 2026 05:47
@sanskar-singh-2403

Copy link
Copy Markdown
Author

hey, @oavdeev @ankurs just a friendly reminder... waiting for your review(s) 🙏

@codecov-commenter

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 88.23529% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 45.96%. Comparing base (294efad) to head (621c1b1).

Files with missing lines Patch % Lines
sdk/python/feast/utils.py 88.23% 1 Missing and 1 partial ⚠️
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6622      +/-   ##
==========================================
+ Coverage   45.94%   45.96%   +0.01%     
==========================================
  Files         412      412              
  Lines       48864    48879      +15     
  Branches     6913     6915       +2     
==========================================
+ Hits        22452    22465      +13     
- Misses      24859    24860       +1     
- Partials     1553     1554       +1     
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 47.26% <88.23%> (+0.01%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/utils.py 77.07% <88.23%> (+0.21%) ⬆️

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 294efad...621c1b1. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

when calling for several ODFVs at once, only fetches one ODFV's column-list per shared source view (last request wins),

2 participants