Skip to content

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

Merged
ntkathole merged 1 commit into
feast-dev:masterfrom
sanskar-singh-2403:fix/odfv-shared-source-projection-merge
Jul 30, 2026
Merged

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

Conversation

@sanskar-singh-2403

Copy link
Copy Markdown
Contributor

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
@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
Contributor Author

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

@codecov-commenter

codecov-commenter commented Jul 22, 2026

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 95.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 46.37%. Comparing base (0f149a9) to head (b4feda7).

Files with missing lines Patch % Lines
sdk/python/feast/utils.py 95.00% 1 Missing ⚠️
❗ 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   46.35%   46.37%   +0.01%     
==========================================
  Files         414      414              
  Lines       50052    50069      +17     
  Branches     7151     7153       +2     
==========================================
+ Hits        23201    23217      +16     
- Misses      25229    25230       +1     
  Partials     1622     1622              
Flag Coverage Δ
go-feature-server 30.58% <ø> (ø)
python-unit 47.67% <95.00%> (+0.01%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/utils.py 76.94% <95.00%> (+0.42%) ⬆️

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 0f149a9...b4feda7. 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.

@sanskar-singh-2403

Copy link
Copy Markdown
Contributor Author

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

hey @oavdeev @ankurs @ches just a friendly reminder to review this PR, thanks!

cc: @addenergyx

Comment thread sdk/python/feast/utils.py Outdated

@addenergyx addenergyx left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! lgtm

@sanskar-singh-2403
sanskar-singh-2403 force-pushed the fix/odfv-shared-source-projection-merge branch 3 times, most recently from d93d559 to ea94468 Compare July 25, 2026 22:00
@sanskar-singh-2403

Copy link
Copy Markdown
Contributor Author

@franciscojavierarceo @ntkathole requesting your reviews in this, Thanks!

@ntkathole
ntkathole force-pushed the fix/odfv-shared-source-projection-merge branch from ea94468 to b4feda7 Compare July 30, 2026 06:26
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. 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.

Adds TestGetFeatureViewsToUseSharedSource in tests/unit/test_utils.py
covering the merged-projection result and order independence.

Fixes feast-dev#6621

Signed-off-by: Sanskar Singh <sanskarsinghty1234@gmail.com>
@ntkathole
ntkathole force-pushed the fix/odfv-shared-source-projection-merge branch from b4feda7 to da47014 Compare July 30, 2026 08:30
@ntkathole
ntkathole merged commit d269946 into feast-dev:master Jul 30, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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),

4 participants