Skip to content

fix: support SQL string as entity_df in RemoteOfflineStore.get_historical_features#6265

Open
korbonits wants to merge 1 commit intofeast-dev:masterfrom
korbonits:fix/remote-offline-store-sql-entity-df
Open

fix: support SQL string as entity_df in RemoteOfflineStore.get_historical_features#6265
korbonits wants to merge 1 commit intofeast-dev:masterfrom
korbonits:fix/remote-offline-store-sql-entity-df

Conversation

@korbonits
Copy link
Copy Markdown

@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

…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>
@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

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.

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

1 participant