GeezerVison

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

GeezerVison is showing github.com — fix: Keep an ODFV output that shares its name with a source feature by luc-pimentel · Pull Request #7009 · feast-dev/feast

Conversation

luc-pimentel commented Oct 14, 2026

Copy link

Copy Markdown

What this PR does / why we need it:

Since 0.62.0 (#6140) get_historical_features fetches every feature of an on demand feature view's source view, requested or not. When one of those source features has the same name as the ODFV output, which schema inference over a wider table produces without anyone noticing, the result is wrong in two ways:

default names: the frame has two revenue_plus_one columns, the source value and the transformed one
full_feature_names=True: only customer_stats__revenue_plus_one comes back and the ODFV output is silently missing. get_online_features drops it the same way.

0.51.0 through 0.61.0 return the transformed value once in both modes.

Two mechanisms. RetrievalJob.to_arrow appended the ODFV column next to the same-named source column. With full names, transform_arrow and transform_dict copy the source column to a short-name alias for the UDF, the UDF returns its output under that same short name, and the alias cleanup drops it.

The fix skips the ODFV's own output names in the alias cleanup (all three transform paths) and makes to_arrow replace a same-named column instead of appending a second one. The unrequested-output filter from #6140 is untouched.

Unrequested source-view features are still returned with the result, as they have been since 0.62.0; that is left as is here and may deserve its own issue. Open PR #6900 touches the same cleanup loop for python-mode full names online and composes with this change.

Which issue(s) this PR fixes:

Fixes #5534

Checks

I've made sure the tests are passing.
My commits are signed off (git commit -s)
My PR title follows conventional commits format

Testing Strategy

Unit tests
Integration tests
Manual tests
Testing is not required for this change

Misc

New tests in tests/unit/test_on_demand_pandas_transformation.py: offline retrieval in pandas and python mode with default and full names, and online retrieval with both naming modes, all with a source feature named like the ODFV output. They fail on master (5 of 6) and pass with the fix.

uv run pytest sdk/python/tests/unit/test_on_demand_pandas_transformation.py -q
15 passed

Before and after on the setup above (a parquet with revenue and revenue_plus_one = -999, a FeatureView declaring both, a pandas ODFV computing revenue_plus_one = revenue + 1, request customer_stats:revenue and revenue_plus_one:revenue_plus_one):

	default names	`full_feature_names=True`
0.67.0	two `revenue_plus_one` columns: `[-999, -999, -999]` and `[11, 21, 31]`	`customer_stats__revenue_plus_one = [-999, ...]`, ODFV output absent
this branch	`revenue_plus_one = [11, 21, 31]`	`revenue_plus_one__revenue_plus_one = [11, 21, 31]`

Sorry, something went wrong.

Uh oh!

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

All reactions

luc-pimentel added 2 commits

October 13, 2026 22:47


          test: cover an ODFV output that shares its name with a source feature

6f0f161

Historical and online retrieval of an on demand feature view whose output
has the same name as a feature of its source view, in pandas and python
mode. Fails on master since 0.62.0: the frame carries two columns under
that name with default names, and with full_feature_names=True the source
value comes back alone.

Signed-off-by: luc-pimentel <[email protected]>


          fix: Keep an ODFV output that shares its name with a source feature

08cf3ac

Since feast-dev#6140 get_historical_features fetches every feature of an on demand
feature view's source view, so a source feature named like the ODFV output
reaches the retrieval. RetrievalJob.to_arrow then appended the ODFV column
next to it, and with full_feature_names=True the alias cleanup in
transform_arrow and transform_dict dropped the UDF output because it carried
the same short name as the temporary alias.

The alias cleanup now skips the ODFV's own output names, and to_arrow
replaces a same-named column instead of appending a second one. Online
retrieval goes through the same transform methods.

Fixes feast-dev#5534

Signed-off-by: luc-pimentel <[email protected]>

luc-pimentel requested review from a team as code owners

October 14, 2026 01:52

luc-pimentel requested review from franciscojavierarceo, haoxu0 and nquinn408 and removed request for a team

October 14, 2026 01:52

luc-pimentel mentioned this pull request

Oct 14, 2026

OnDemandFeatureView silently skips transformation if BatchSource defines column with same name #5534

Open

haoxu0 added the ok-to-test label

Oct 15, 2026

luc-pimentel commented Oct 22, 2026

Copy link

Copy Markdown

Author

Could someone please take a look at this PR when you have a chance? Thanks!

All reactions

Sorry, something went wrong.

Uh oh!

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

haoxu0 commented Oct 23, 2026

Copy link

Copy Markdown

Collaborator

Thanks for the fix — the collision regression tests cover the ODFV-only output path well. One additional case seems important before merging: what happens when a caller explicitly requests both driver_hourly_stats:conv_rate_plus_one and plus_one_view:conv_rate_plus_one in the same retrieval (especially with full_feature_names=True)? The current tests request driver_hourly_stats:conv_rate plus the ODFV output, so they don't establish whether both explicitly requested values remain accessible. In RetrievalJob.to_arrow, set_column() replaces the first same-named source column, which could discard an explicitly requested source value in the default-name path. Could we add an offline + online regression for the dual-request case, and clarify the intended collision policy for full_feature_names=False vs True? It would also be useful to exercise the Ibis transformation path since _without_output_names changes that cleanup path too.

All reactions

Sorry, something went wrong.

Uh oh!

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


          test: Cover both refs requested and the ibis path under an ODFV name …

fdcd851

…collision

Requesting the source feature and the ODFV output under one default name
raises FeatureNameCollisionError before retrieval, offline and online, as
before. With full_feature_names=True both come back under their own names;
on master the ODFV output is dropped. The ibis transformation path keeps
the output under the same collision, exercised through the ibis function
without a substrait plan.

Signed-off-by: luc-pimentel <[email protected]>

luc-pimentel commented Oct 23, 2026

Copy link

Copy Markdown

Author

Thanks for the look. Requesting both driver_hourly_stats:conv_rate_plus_one and plus_one_view:conv_rate_plus_one with default names raises FeatureNameCollisionError from _validate_feature_refs before anything is fetched, on master and on this branch. That check runs on the requested refs before the source view's refs are added, so set_column never sees two explicitly requested values under one name. With full_feature_names=True both come back under their own names, driver_hourly_stats__conv_rate_plus_one with the source value and plus_one_view__conv_rate_plus_one with the transformed one; on master the ODFV output is dropped there, offline and online. So the policy this branch keeps is the existing one: a default-name collision between requested refs is an error, and full names make both reachable. I pushed tests for the dual request offline and online in both naming modes, plus two through transform_ibis on a substrait ODFV, run through the ibis function since CI has no ibis-substrait.

All reactions

Sorry, something went wrong.

Uh oh!

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

luc-pimentel mentioned this pull request

Nov 7, 2026

fix: Return python mode ODFV features with full feature names #6900

Open

4 of 7 tasks

This branch has not been deployed

No deployments

Reviewers

franciscojavierarceo

Awaiting requested review from franciscojavierarceo

franciscojavierarceo is a code owner automatically assigned from feast-dev/maintainers

haoxu0

Awaiting requested review from haoxu0

haoxu0 is a code owner automatically assigned from feast-dev/maintainers

nquinn408

Awaiting requested review from nquinn408

nquinn408 is a code owner automatically assigned from feast-dev/maintainers

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.

OnDemandFeatureView silently skips transformation if BatchSource defines column with same name

Uh oh!

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

2 participants

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.