Skip to content

fix: Preserve all join keys in remote online store read requests#6603

Open
AmosBunde wants to merge 1 commit into
feast-dev:masterfrom
AmosBunde:fix/remote-online-read-composite-join-keys
Open

fix: Preserve all join keys in remote online store read requests#6603
AmosBunde wants to merge 1 commit into
feast-dev:masterfrom
AmosBunde:fix/remote-online-read-composite-join-keys

Conversation

@AmosBunde

@AmosBunde AmosBunde commented Jul 13, 2026

Copy link
Copy Markdown

What this PR does / why we need it:

RemoteOnlineStore._construct_online_read_api_json_request built the request body from row.join_keys[0] and row.entity_values[0] only. For a feature view with a composite entity key, every join key after the first was dropped before the request left the client. The feature server then joined on a partial key, matched a row, and returned it with status PRESENT. No exception was raised anywhere in the path, so callers silently received features belonging to the wrong entity.

Reproduced on 0.64.0 with a two-entity feature view keyed on (driver_id, customer_id). The request that reached the server was:

{'features': ['driver_customer_stats:conv_rate'],
 'entities': {'driver_id': [1001, 1002]}}

customer_id is not null or truncated. It is absent.

Two notes for reviewers:

  1. The issue title attributes this to get_online_features, but that method already builds a correct columnar request across all keys. The defect is one layer down, in the online_read path taken when a local FeatureStore is configured with an online store of type: remote.

  2. get_online_features is covered by test_columnar_entity_rows. The online_read request builder had no test coverage at all, which is why the divergence between the two went unnoticed.

This PR builds the request from every (join_key, entity_value) pair, keyed on join key name rather than position, and raises on a length mismatch between join_keys and entity_values rather than silently truncating.

Which issue(s) this PR fixes:

Fixes #6488

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

@AmosBunde
AmosBunde requested a review from a team as a code owner July 13, 2026 18:16
@AmosBunde
AmosBunde force-pushed the fix/remote-online-read-composite-join-keys branch from a2a5eac to 10fb95f Compare July 13, 2026 22:44
)
for join_key, value in zip(row.join_keys, row.entity_values):
entity_columns.setdefault(join_key, []).append(
getattr(value, value.WhichOneof("val")) # type: ignore[arg-type]

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

WhichOneof("val") returns None for an unset ValueProto, so getattr(value, None) raises TypeError here. The issue specifically calls out null entity values; this should append None instead.

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 on remote stores drops all join keys but the first

2 participants