Skip to content

fix: Use a shared entity_rows name as both request data and join key - #6794

Open
OHMFOHS wants to merge 1 commit into
feast-dev:masterfrom
OHMFOHS:fix/shared-request-data-join-key
Open

fix: Use a shared entity_rows name as both request data and join key#6794
OHMFOHS wants to merge 1 commit into
feast-dev:masterfrom
OHMFOHS:fix/shared-request-data-join-key

Conversation

@OHMFOHS

@OHMFOHS OHMFOHS commented Aug 28, 2026

Copy link
Copy Markdown

What this PR does

Closes #6790.

_prepare_entities_to_read_from_online_store classified each entity_rows key with a mutually-exclusive if/elif chain:

if join_key_or_entity_name in needed_request_data:
    request_data_features[join_key_or_entity_name] = values
elif join_key_or_entity_name in join_keys_set:
    ...

A name present in needed_request_data was therefore only ever treated as request data. When an OnDemandFeatureView's RequestSource declares a field whose name matches a FeatureView's join key, the join-key branch is unreachable, join_key_values never receives the value, and every FeatureView expecting that join key fails with a missing-join-key error — even though the value was present in entity_rows all along.

get_historical_features accepts the equivalent entity_df, so the same input succeeds offline and fails online.

This PR classifies the two roles independently.

A second issue this surfaces

Making that name reachable in both mappings exposes a latent failure one line later:

data=dict(**join_key_values, **request_data_features)

dict(**a, **b) raises TypeError: got multiple values for keyword argument when the same key is in both mappings — exactly the case this change makes possible. Switched to the dict literal {**a, **b}, which permits the duplicate. Both mappings hold the same values object for a shared name, so the merge is unambiguous. test_shared_name_populates_response_once guards against the name being emitted twice.

Tests

Adds sdk/python/tests/unit/test_shared_request_data_join_key.py (5 cases), following the existing test_feature_resolution_cache.py pattern of patching _get_online_request_context:

  • the shared name resolves as a join key
  • the shared name appears in the result row names
  • the shared name is emitted exactly once in the response
  • a request-data-only name does not leak into join_key_values (control)
  • an ordinary join key is unchanged (control)

Two of these fail on master (assert 'user_id' in {}join_key_values is empty) and all five pass with this change.

Verification

Full sdk/python/tests/unit run, same environment, before and after:

failed passed
master 94 1420
this branch 94 1425

The failure sets are identical in both directions — the 94 pre-existing failures are unrelated collection/dependency errors in my local environment (missing optional extras such as httpx2, boto3, pymilvus), and the +5 is exactly the new tests.

ruff check, ruff format --check, and mypy feast/utils.py are all clean on the changed files.

_prepare_entities_to_read_from_online_store classified each entity_rows
key with a mutually-exclusive if/elif chain, so a name that appears in
needed_request_data was only ever treated as request data. When an
OnDemandFeatureView's RequestSource declares a field whose name matches a
FeatureView's join key, the join key branch was unreachable and every
FeatureView expecting that join key failed with a missing-join-key error,
even though the value was present in entity_rows.

get_historical_features accepts the equivalent entity_df, so the same
input succeeded offline and failed online.

Classify the request-data and join-key roles independently. Building the
response also had to move from dict(**a, **b) to {**a, **b}: the former
raises TypeError when the same name is present in both mappings, which is
exactly the case this change makes reachable.

Adds five regression tests covering the shared name resolving as a join
key, appearing in the result row names, being emitted only once in the
response, and the two unchanged control cases.

Closes feast-dev#6790

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Signed-off-by: Junchao Mao <925160568@qq.com>
@OHMFOHS
OHMFOHS requested a review from a team as a code owner August 28, 2026 06:18
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.

get_online_features silently drops a join key value when it shares a name with an OnDemandFeatureView's RequestSource field

1 participant