feat: Add opt-in filter_by_created_timestamp cutoff to get_historical_features#6617
Open
addenergyx wants to merge 8 commits into
Open
feat: Add opt-in filter_by_created_timestamp cutoff to get_historical_features#6617addenergyx wants to merge 8 commits into
addenergyx wants to merge 8 commits into
Conversation
…ical_features When a feature view has a created_timestamp_column, it is currently used only as a dedup tiebreaker in point-in-time joins, so retrieval can serve feature values whose created_timestamp is after the entity row's event timestamp (backfills, late corrections). This leaks future information into training data and makes training sets non-reproducible. Add an opt-in at_event_time flag (default False) to get_historical_features that adds a created_timestamp <= entity_timestamp predicate to the point-in-time join, so retrieval reflects what was known as of each entity row's timestamp. Implemented for the SQL template stores (BigQuery, Redshift, Spark, Trino, Athena, Postgres, ClickHouse, Couchbase), the ibis-based stores (DuckDB, MSSQL, Oracle) and the dask store. Snowflake, Remote and Ray raise NotImplementedError when the flag is set. Fixes feast-dev#6615 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk>
Adds a universal offline store integration test covering at_event_time (default returns backfilled values, at_event_time=True only returns values created at or before the entity timestamp, stores without support skip via NotImplementedError) and documents the flag on the point-in-time joins concept page. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk>
…ress review feedback Rename the flag to filter_by_created_timestamp to match the repo's mechanical flag naming and sit alongside created_timestamp_column. Review fixes: - HybridOfflineStore now forwards optional kwargs to the delegated store instead of raising TypeError for supported underlying stores. - The dask filter now excludes feature rows with a null created timestamp (consistent with the SQL predicate) while still keeping unmatched entity rows from the left join. - The integration test only treats NotImplementedError from get_historical_features itself as an unsupported-store skip. - Add template-render tests for all SQL dialects and a dask null created-timestamp test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk>
…re capability flag Simplification pass over the feature, reviewed again by Codex: - Declare support via OfflineStore.supports_filter_by_created_timestamp (default False) and check it once in the passthrough provider via ensure_filter_by_created_timestamp_supported, replacing the three per-store NotImplementedError guards. Unsupported stores can no longer silently ignore the flag. - The hybrid store re-checks the resolved child store before delegating. - Standardize every supporting store on an explicit filter_by_created_timestamp parameter instead of kwargs.get lookups. - Document the flag in the OfflineStore.get_historical_features docstring alongside start_date/end_date. - Return the dask created-timestamp filter lazily so the mask fuses into the following _drop_duplicates persist instead of forcing an extra materialization. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk>
addenergyx
marked this pull request as ready for review
July 21, 2026 17:07
addenergyx
requested review from
ejscribner,
robhowley and
tokoko
and removed request for
a team
July 21, 2026 17:07
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does / why we need it
Adds an opt-in
filter_by_created_timestamp: bool = Falseparameter toget_historical_featuresthat enforcescreated_timestamp <= entity_timestampin point-in-time joins.Today
created_timestamp_columnis used only as a dedup tiebreaker, never as a filter, so a training row at entity timestampTcan be served a value that did not exist yet atT(backfills, late corrections). Withfilter_by_created_timestamp=True, retrieval reflects what was actually known as of each entity row's timestamp, i.e. what the online store would have served at that moment.Design
FeatureStore.get_historical_featuresusing the same optional-kwargs pattern asstart_date/end_date: only passed to the offline store whenTrue, so the default path is untouched and third-party stores are unaffected unless a user explicitly opts in.build_point_in_time_querygains afilter_by_created_timestamptemplate variable, and each template's__basejoin gains one guarded predicate:ROW_NUMBER) is unchanged.created_timestamp_columnare unaffected.Store coverage
Implemented: BigQuery, Redshift, Spark, Trino, Athena, Postgres, ClickHouse, Couchbase (SQL templates); DuckDB, MSSQL, Oracle (via the shared ibis
point_in_time_join); Dask/file (pandas filter mirroring_filter_ttl); Hybrid (delegates, re-checking the resolved child store).Stores declare support via a
supports_filter_by_created_timestampclass attribute onOfflineStore(defaultFalse). A single guard in the passthrough provider raisesNotImplementedErrorwhen the flag is set for a store that does not declare support, so no store can silently ignore it and new stores are covered automatically. Unsupported today: Snowflake (its ASOF JOIN dedups before the join andMATCH_CONDITIONtakes a single predicate, so the cutoff cannot be expressed without restructuring the query), Remote (needs protocol support to forward the flag), Ray, and MongoDB.Notes
created_timestampare excluded by the predicate in all implementations; the docstring notes the column should be non-null when the mode is used. (In the dask store, unmatched entity rows from the left join are still preserved.)ttl: TTL bounds by event time, this bounds by known time.Which issue(s) this PR fixes
Fixes #6615
Misc
Tests added in
sdk/python/tests/unit/infra/offline_stores/test_filter_by_created_timestamp.py:filter_by_created_timestamp=Trueand acreated_timestamp_columnis set; default rendering unchanged.filter_by_created_timestamp=Trueserves the version known at the entity timestamp.ruff check/ruff formatclean;mypy feastreports the same 17 pre-existing errors asmaster(none introduced).