Skip to content

fix: Support sql string as entity_df in remote offline store#6265

Merged
ntkathole merged 1 commit intofeast-dev:masterfrom
korbonits:fix/remote-offline-store-sql-entity-df
Apr 16, 2026
Merged

fix: Support sql string as entity_df in remote offline store#6265
ntkathole merged 1 commit intofeast-dev:masterfrom
korbonits:fix/remote-offline-store-sql-entity-df

Conversation

@korbonits
Copy link
Copy Markdown
Contributor

@korbonits korbonits commented Apr 11, 2026

Closes #6236

Summary

Fixes AttributeError: 'str' object has no attribute 'columns' when passing a SQL string as entity_df to RemoteOfflineStore.get_historical_features().

  • remote.py: If entity_df is a str, stash it as api_parameters["entity_df_sql"] and set entity_df=None before building the RemoteRetrievalJob. Also fixes _create_retrieval_metadata to return a stub (empty keys, no timestamps) for SQL strings instead of crashing on .columns.
  • offline_server.py: After the existing empty-table detection, check for entity_df_sql in the command dict and forward it as-is to the backing offline store.

Local offline stores (ClickHouse, PostgreSQL, BigQuery, DuckDB) already support SQL entity_df; this fix makes RemoteOfflineStore a transparent pass-through for that case.

Test plan

  • test_create_retrieval_metadata_with_sql_string — verifies the metadata helper returns a valid stub for a SQL string
  • test_remote_offline_store_sql_entity_df_routing — verifies the client moves a SQL string into api_parameters["entity_df_sql"] and passes entity_df=None to the job
  • test_offline_server_get_historical_features_passes_sql_to_store — verifies the server forwards entity_df_sql as a string to the backing store
  • All existing test_offline_server.py tests pass (7/7)

🤖 Generated with Claude Code


Open with Devin

@korbonits korbonits requested a review from a team as a code owner April 11, 2026 18:37
@korbonits korbonits marked this pull request as draft April 11, 2026 18:37
@korbonits korbonits marked this pull request as ready for review April 11, 2026 18:40
Copy link
Copy Markdown
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

@korbonits korbonits changed the title fix: support SQL string as entity_df in RemoteOfflineStore.get_historical_features fix: support SQL string as entity_df in remote offline store Apr 12, 2026
@korbonits korbonits changed the title fix: support SQL string as entity_df in remote offline store fix: support sql string as entity_df in remote offline store Apr 13, 2026
@ntkathole ntkathole changed the title fix: support sql string as entity_df in remote offline store fix: Support sql string as entity_df in remote offline store Apr 14, 2026
@ntkathole ntkathole force-pushed the fix/remote-offline-store-sql-entity-df branch from 137e93f to 9c90283 Compare April 14, 2026 04:48
@korbonits korbonits force-pushed the fix/remote-offline-store-sql-entity-df branch from 9c90283 to f732d31 Compare April 15, 2026 04:33
@ntkathole ntkathole force-pushed the fix/remote-offline-store-sql-entity-df branch from f732d31 to f0ebeef Compare April 16, 2026 05:57
…ical_features

When entity_df is a SQL string, route it through api_parameters["entity_df_sql"]
instead of calling pa.Table.from_pandas() on it (which raised AttributeError).
The OfflineServer now forwards entity_df_sql from the command dict directly to
the backing offline store, matching the behaviour of local stores that already
support SQL entity_df.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Signed-off-by: Alex Korbonits <alex@korbonits.com>
@ntkathole ntkathole force-pushed the fix/remote-offline-store-sql-entity-df branch from f0ebeef to 1bd2893 Compare April 16, 2026 14:46
@ntkathole ntkathole merged commit c559889 into feast-dev:master Apr 16, 2026
16 of 26 checks 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.

RemoteOfflineStore does not support SQL string as entity_df in get_historical_features()

3 participants