feat: Add opt-in filter_by_created_timestamp cutoff to get_historical_features - #6617
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>
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>
franciscojavierarceo
left a comment
There was a problem hiding this comment.
The Dask implementation can drop entity rows. It performs the left merge first and then _filter_created_timestamp() removes rows whose matched feature has created_timestamp > entity_timestamp. If every candidate for an entity is too new, the merge produced no unmatched/null row to fall back to, so filtering removes that entity entirely. get_historical_features() should preserve entity-dataframe cardinality and return null features in this case. Please apply the cutoff in/before the join or reconstruct unmatched rows, and add a regression where all matching candidates are future-created.
| feature_table[timestamp_field] <= entity_table[event_timestamp_col], | ||
| ) | ||
|
|
||
| if filter_by_created_timestamp and created_timestamp_field: |
There was a problem hiding this comment.
This predicate casts the created timestamp to UTC but the existing event-timestamp comparison on the same join doesn't cast. Compare with the existing predicate at line 433-434 which does no cast.
There was a problem hiding this comment.
Thanks @ntkathole, made a change so read_fv now normalizes the created timestamp on read, as it already did for the event timestamp, so the predicate no longer casts. Made it unconditional since deduplicate() reads that column regardless of the flag.
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #6617 +/- ##
==========================================
+ Coverage 46.46% 46.74% +0.28%
==========================================
Files 414 414
Lines 50138 50173 +35
Branches 7173 7179 +6
==========================================
+ Hits 23295 23454 +159
+ Misses 25204 25079 -125
- Partials 1639 1640 +1
... and 3 files with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
The Dask cutoff filtered rows after the left join, so an entity whose every candidate version was created after its timestamp lost all of its rows and disappeared from the result. get_historical_features must preserve entity-dataframe cardinality and return null features instead. Blank the feature-view columns rather than dropping the row, which leaves it indistinguishable from an unmatched left join. _drop_duplicates already sorts nulls first and keeps the last row, so a valid version still wins where one exists and a blanked row survives only when nothing else does. The unit tests now set fv.entity_columns. Without it the derived join keys are empty and _merge silently cross joins, which hid the per-entity behaviour because the existing cases all used a single entity row. Signed-off-by: David <david-adeniji@hotmail.co.uk>
The cutoff predicate cast created_timestamp to UTC while the event-timestamp comparison beside it did not. read_fv normalizes the event timestamp when it reads the source and left the created timestamp alone, so the predicate was compensating for a missing normalization at the one comparison site. Normalize both on read instead. The predicate then needs no cast and matches its neighbour. deduplicate() orders by created_timestamp_column regardless of the cutoff flag, so the normalization is unconditional rather than gated on it; casting a column that is already tz-aware compiles away, so this leaves the emitted query unchanged for tz-aware sources and retires the "mutate only if tz-naive" TODO. Signed-off-by: David <david-adeniji@hotmail.co.uk>
Signed-off-by: David <david-adeniji@hotmail.co.uk>
|
Thanks @franciscojavierarceo should be fixed now, it now nulls out the feature columns instead of dropping the row, so the entity row survives with null features |
…_features (feast-dev#6617) * feat: Add opt-in at_event_time created_timestamp cutoff to get_historical_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> * test: Add universal integration test and docs for at_event_time 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> * refactor: Rename at_event_time to filter_by_created_timestamp and address 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> * refactor: Centralize filter_by_created_timestamp support behind a store 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> * chore: Trim comments to the non-obvious constraints Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk> * docs: Tighten filter_by_created_timestamp docstring Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk> * docs: Condense filter_by_created_timestamp caveats into a hint Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: David <david-adeniji@hotmail.co.uk> * fix: Keep entity rows whose candidate versions are all future-created The Dask cutoff filtered rows after the left join, so an entity whose every candidate version was created after its timestamp lost all of its rows and disappeared from the result. get_historical_features must preserve entity-dataframe cardinality and return null features instead. Blank the feature-view columns rather than dropping the row, which leaves it indistinguishable from an unmatched left join. _drop_duplicates already sorts nulls first and keeps the last row, so a valid version still wins where one exists and a blanked row survives only when nothing else does. The unit tests now set fv.entity_columns. Without it the derived join keys are empty and _merge silently cross joins, which hid the per-entity behaviour because the existing cases all used a single entity row. Signed-off-by: David <david-adeniji@hotmail.co.uk> * refactor: Normalize created timestamp on read, not in the join predicate The cutoff predicate cast created_timestamp to UTC while the event-timestamp comparison beside it did not. read_fv normalizes the event timestamp when it reads the source and left the created timestamp alone, so the predicate was compensating for a missing normalization at the one comparison site. Normalize both on read instead. The predicate then needs no cast and matches its neighbour. deduplicate() orders by created_timestamp_column regardless of the cutoff flag, so the normalization is unconditional rather than gated on it; casting a column that is already tz-aware compiles away, so this leaves the emitted query unchanged for tz-aware sources and retires the "mutate only if tz-naive" TODO. Signed-off-by: David <david-adeniji@hotmail.co.uk> * Shorten comments in the created timestamp cutoff Signed-off-by: David <david-adeniji@hotmail.co.uk> --------- Signed-off-by: David <david-adeniji@hotmail.co.uk> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
# [0.66.0](v0.65.0...v0.66.0) (2026-08-21) ### Bug Fixes * Add connection pre-warming for DynamoDB async client ([89240fa](89240fa)), closes [#6060](#6060) * Add remote registry client extra ([#6697](#6697)) ([b8dfcb0](b8dfcb0)) * Address review feedback on FIPS cipher suite configuration ([4a35fba](4a35fba)) * Allow remote-registry first apply for new projects ([39d408d](39d408d)) * Avoid importing feast.feature_store at mcp_server import time ([ddb2e9a](ddb2e9a)) * Bump pymssql to >=2.3.6 for macOS arm64 wheel support ([181eb35](181eb35)), closes [#5636](#5636) [#5193](#5193) [#5636](#5636) * Call ApplySavedDataset RPC instead of ApplyFeatureService in RemoteRegistry.apply_saved_dataset() ([934d341](934d341)) * Catch missing dbt parser dependency in dbt CLI commands ([#6534](#6534)) ([3c2ae3c](3c2ae3c)) * Default authentication to kubernetes auth ([6a4690a](6a4690a)) * Defer feature-freshness thread to post-fork to avoid Gunicorn deadlock ([#6648](#6648)) ([104ad10](104ad10)), closes [#6647](#6647) * Do not pass undeclared feature view columns to ODFV UDFs ([#6527](#6527)) ([75b9463](75b9463)) * downgrade mcp pin to 1.29.0 and fix CI lockfiles and unit tests ([98e5bca](98e5bca)), closes [#6706](#6706) * Feast apply silently ignoring ttl updates to None or timedelta(0) ([#6709](#6709)) ([97b0f25](97b0f25)), closes [#6703](#6703) * Fix mypy TorchTensor type alias error ([#6712](#6712)) ([34de6fa](34de6fa)), closes [#5563](#5563) * Fixed data source creation form gaps ([5d0f7d6](5d0f7d6)) * Handle parameterized and complex Trino types in type map ([326554d](326554d)) * Isolate default user permissions ([e37adbf](e37adbf)) * Isolate projection join key maps ([d1c709d](d1c709d)) * Map Postgres real to FLOAT instead of DOUBLE ([62db435](62db435)) * Merge shared ODFV source projections in feature resolution ([d269946](d269946)), closes [#6621](#6621) * More exhaustive athena types ([a9aaefc](a9aaefc)) * Normalize SQL registry read_path to the psycopg3 driver like path ([#6644](#6644)) ([996c6ea](996c6ea)), closes [#6643](#6643) * **operator:** add spec.services.onlineStore.disabled to opt out of the online store ([d81d4e3](d81d4e3)), closes [#6586](#6586) * Preinstall DuckDB delta extension for tests ([fd4d49d](fd4d49d)), closes [#6743](#6743) * Preserve event-time ordering within Redis online_write_batch ([40fb788](40fb788)), closes [#5163](#5163) * Prevent mutation of cached feature resolution results ([ea17419](ea17419)) * Remote feastRef FeatureStore fails first apply for a new feastProject ([9affee5](9affee5)) * Remove inert subjectaccessreviews and reorganize RBAC rules ([f771ea4](f771ea4)) * Report single-feature-view spark_application materialization success ([a9219d9](a9219d9)), closes [#6673](#6673) * Reset the global security manager after the permissions fixture ([7667215](7667215)) * Resolve kserve with pip --dry-run instead of installing it ([01da132](01da132)), closes [#6732](#6732) * Resolve write_to_offline_store feature view with a single registry lookup ([a42dc85](a42dc85)), closes [#4235](#4235) * Return False from __eq__ on cross-type comparison ([#6637](#6637)) ([0f149a9](0f149a9)), closes [#6636](#6636) * Reuse IdP-issued client tokens until near expiry ([602d752](602d752)) * Reuse the OIDC JWKS client across requests ([#6683](#6683)) ([a1e6fc2](a1e6fc2)) * Separate CronJob and feature-server ServiceAccounts ([398f643](398f643)) * Serialize UnixTimestamp proto values as raw int64 in remote online store transport ([1e7134f](1e7134f)) * Set FIPS cipher suites before pyarrow.flight import to prevent crash on IBM Power ([979b82a](979b82a)) * Support Entra ID (Azure AD) token claims in OIDC auth ([#6631](#6631)) ([f843c63](f843c63)) * UDF/ODFV source rehydrate (+ Postgres / online cache) ([#6655](#6655)) ([5fd7af7](5fd7af7)) * Updated projects-list.json in order to display newly added projects ([#6657](#6657)) ([3a6a103](3a6a103)) * Use correct image name in multi-arch imagetools push step ([faf85e0](faf85e0)) * Use join keys instead of entity names in ODFV materialization ([#6645](#6645)) ([abffebc](abffebc)), closes [#5965](#5965) * use matching proto class per feature view list in SqliteOnlineStore.plan() ([adb8c1c](adb8c1c)), closes [#6658](#6658) * Widen Athena integer type mapping for unsigned ints ([3425783](3425783)) ### Features * Add ConnectionRef to DataSource for pluggable external credential resolution ([28bde01](28bde01)) * Add Feature Service Create in UI ([0399380](0399380)) * Add hybrid to ValidOfflineStoreDBStorePersistenceTypes for HybridOfflineStore support ([#6707](#6707)) ([310ab51](310ab51)), closes [#6701](#6701) * Add MLflow integration support to Feast operator ([#6611](#6611)) ([52999f1](52999f1)) * Add opt-in filter_by_created_timestamp cutoff to get_historical_features ([#6617](#6617)) ([79b33ce](79b33ce)), closes [#6615](#6615) * Add optional OIDC token audience and issuer verification ([#6670](#6670)) ([ef307c6](ef307c6)) * Add packaged feature repository support to Feast Operator ([8112b1e](8112b1e)), closes [#6598](#6598) * add plan() support to DynamoDBOnlineStore ([51ce982](51ce982)), closes [#6658](#6658) [#6659](#6659) * Added optional namespace/colleciton to datasets ([165fcf2](165fcf2)) * Added SQL registry schema_mode and registry create command ([#6704](#6704)) ([037c4cd](037c4cd)) * Allow users to have protected project on shared registry ([f9923bc](f9923bc)) * Apply Intermediate TLS defaults on API fallback and handle transient errors ([#6587](#6587)) ([43ae993](43ae993)) * **cli:** Updated feast init demo by adding rag template ([#5946](#5946)) ([c8628eb](c8628eb)), closes [#5264](#5264) * Expose the OIDC JWKS tunables through the operator ([#6690](#6690)) ([fef4e78](fef4e78)), closes [#6683](#6683) * Making feast vector store with open ai search api compatible ([#6121](#6121)) ([54da19a](54da19a)) * Multi-arch publish for feast operator image ([b221036](b221036)) * OpenLineage lineage enhancements - full object coverage, richer UI, and API-level sync ([#6719](#6719)) ([120a868](120a868)) * **operator:** Add spec.services.initImage for init container image override ([#6598](#6598)) ([ca355cb](ca355cb)) * Pass optional OIDC audience and issuer through the operator ([#6677](#6677)) ([a13ed7b](a13ed7b)), closes [#6670](#6670) * **server:** Remote Materialization ([#6649](#6649)) ([b7ae488](b7ae488)), closes [#4526](#4526) * Support Lineage configs via operator ([bf1e54a](bf1e54a)) * Updated datasets UI to support grouping ([7ae64ec](7ae64ec))
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).