GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix: Merge shared ODFV source projections in feature resolution by sanskar-singh-2403 · Pull Request #6622 · feast-dev/feast

Conversation

@sanskar-singh-2403

@sanskar-singh-2403 sanskar-singh-2403 commented Jul 22, 2026

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@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

sanskar-singh-2403 commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@codecov-commenter

codecov-commenter commented Jul 22, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@sanskar-singh-2403

sanskar-singh-2403 commented Jul 23, 2026

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

addenergyx
addenergyx reviewed Jul 23, 2026
View reviewed changes
Comment thread sdk/python/feast/utils.py Resolved

Uh oh!

There was an error while loading. Please reload this page.

@sanskar-singh-2403
sanskar-singh-2403 requested a review from addenergyx July 24, 2026 07:48
addenergyx
addenergyx approved these changes Jul 24, 2026
View reviewed changes

@addenergyx addenergyx left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks! lgtm

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@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

sanskar-singh-2403 commented Jul 29, 2026

Copy link
Copy Markdown
Contributor Author

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

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ntkathole
ntkathole approved these changes Jul 30, 2026
View reviewed changes
@ntkathole
ntkathole force-pushed the fix/odfv-shared-source-projection-merge branch from ea94468 to b4feda7 Compare July 30, 2026 06:26
@ntkathole ntkathole added the ok-to-test label Jul 30, 2026
@sanskar-singh-2403 @ntkathole
fix: Merge shared ODFV source projections in feature resolution
da47014
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 <[email protected]>
@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

Uh oh!

There was an error while loading. Please reload this page.

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

Reviewers

@ntkathole ntkathole ntkathole approved these changes
+1 more reviewer
@addenergyx addenergyx addenergyx approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

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

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@sanskar-singh-2403 @codecov-commenter @ntkathole @addenergyx
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.