nit: Since only end_date is used from **kwargs, consider accepting end_date as an explicit keyword argument instead of relying on **kwargs. This makes the API self-documenting and easier to type-check:

def get_historical_features(
    ...,
    full_feature_names: bool = False,
    end_date: Optional[datetime] = None,
) -> RetrievalJob:

Though if start_date support is planned, keeping **kwargs for now is fine.

Only end_date is extracted from kwargs, but start_date is not. The Postgres store supports both start_date and end_date and has a dedicated {% if start_date and end_date %} branch in its query template for range-based retrieval.

Without start_date support, users can't restrict how far back the PIT join looks (beyond TTL). This means this isn't full parity with Postgres — it's single-point retrieval at end_date, not range-based retrieval over [start_date, end_date].

Consider extracting and forwarding start_date to the query builder in a follow-up, and adding a non-entity branch to the ClickHouse query template.

🔴 Non-entity retrieval silently ignores start_date kwarg unlike Postgres counterpart

When entity_df is None, the Clickhouse get_historical_features only reads end_date from kwargs (clickhouse.py:59) and completely ignores start_date. The caller feature_store.py:1369-1370 passes start_date as a kwarg when the user provides it. The Postgres implementation (postgres.py:132-160), which this code is modeled after, uses start_date to compute the entity_df timestamp and to bound the TTL-based data scan window. In the Clickhouse version, a user-provided start_date is silently dropped, meaning the point-in-time join will use end_date as the sole entity timestamp regardless of the user's intent — potentially returning different (and unexpected) feature data compared to the Postgres offline store for the same inputs.

In sdk/python/feast/infra/offline_stores/contrib/clickhouse_offline_store/clickhouse.py, lines 57-65, add handling for the start_date kwarg to match the Postgres implementation at sdk/python/feast/infra/offline_stores/contrib/postgres_offline_store/postgres.py lines 132-168. Specifically:

1. Before the `if entity_df is None:` block, extract start_date from kwargs: `start_date = kwargs.get("start_date", None)`
2. Inside the block, after computing end_date, add logic to compute start_date from TTL if not provided (matching postgres.py lines 145-160):
   - If start_date is None, find the max TTL across feature_views and set start_date = end_date - max_ttl (or default to 30 days)
   - If start_date is provided, make it tz-aware with make_tzaware(start_date)
3. You will also need to import timedelta from datetime at the top of the file.
4. Consider whether the entity_df should use start_date or end_date as the event_timestamp (the Postgres version uses start_date via pd.date_range[:1], while the current Clickhouse version uses end_date).

Was this helpful? React with 👍 or 👎 to provide feedback.

@YassinNouh21 This seems critical issue.

Not a bug — this is intentional. The PIT join uses MAX(entity_timestamp) as the upper bound, so the timestamp in the synthetic entity_df IS the query upper bound. Using [end_date] gives the window [end_date - TTL, end_date], which is correct. The Postgres implementation using pd.date_range(start=start_date, ...)[:1] actually has the bug — it takes start_date as the sole timestamp, making end_date unreachable. Our implementation matches Dask and is the correct behavior.
@ntkathole

@ntkathole sounds like there's a bug in postgres???

Yes I noticed that

It will get resolved via #6057 cc @Vperiodt

Good call using [end_date] here. The Postgres implementation at postgres.py:162-168 has a subtle bug where it uses:

pd.date_range(start=start_date, end=end_date, freq="1s", tz=timezone.utc)[:1]

[:1] takes the first element which is start_date, so the PIT join upper bound (MAX(entity_timestamp)) becomes start_date instead of end_date — making data after start_date unreachable.

This implementation correctly uses end_date as the entity timestamp, matching Dask's behavior and ensuring the PIT window is [end_date - TTL, end_date].

Tests seems over-mocked

You're right, removed the heavy mocking. For proper coverage, an integration test against a real ClickHouse instance would be more valuable than over-mocked unit tests — the unit tests here may not be necessary at all.

The unit tests verify that a RetrievalJob is returned, but don't assert anything about the generated SQL or the synthetic entity_df contents. Consider adding an assertion that the entity_df passed to _upload_entity_df contains the expected end_date timestamp — this would catch regressions like the Postgres start_date bug.

Also, a test for entity_df=None without any end_date kwarg would verify the _utc_now() default path.