Repository navigation
feast materialize fails after upgrading to 0.50 with clickhouse backend #5493
Description
Expected Behavior
The feast materialize command should work successfully with ClickHouse offline store after upgrading from Feast 0.49 to 0.50, allowing materialization of features to the online store.
Current Behavior
After upgrading from Feast 0.49 to 0.50, the command feast -c feature_repo materialize 2000-04-29 2025-05-01 -v user fails with an NotImplementedError indicating that the ClickHouse offline store does not implement the pull_all_from_table_or_query method.
The ClickHouse offline store implementation only provides pull_latest_from_table_or_query method but the materialization process is attempting to call pull_all_from_table_or_query.
Steps to reproduce
- Set up a Feast feature store with ClickHouse as the offline store
- Upgrade from Feast 0.49 to 0.50
- Run materialization command:
feast -c feature_repo materialize 2000-04-29 2025-05-01 -v user - Observe the AttributeError
Specifications
- Version: 0.50 (custom fork:
git+https://github.com/leiyangyou/feast.git@fix/entity-value-type-mapping-for-aliased-fv-v0.50) - Platform: macOS 24.6.0 (Darwin)
- Subsystem: ClickHouse offline store materialization
- Python: 3.12.9
- Offline Store: ClickHouse (custom implementation in
feast/offline_stores/clickhouse/clickhouse.py)
Possible Solution
The issue appears to be related to changes in the materialization logic between versions 0.49 and 0.50. There are two potential solutions:
-
Implement
pull_all_from_table_or_querymethod in ClickHouse offline store: Add the missing method to theClickhouseOfflineStoreclass to match the interface expected by the materialization process. -
Fix materialization logic to use the correct method: Investigate whether the materialization process should be calling
pull_latest_from_table_or_queryinstead ofpull_all_from_table_or_queryfor ClickHouse offline stores, as this aligns with the typical use case of getting the latest feature values for materialization.
The core question is: Should materialization use pull_all_from_table_or_query or pull_latest_from_table_or_query?
For most materialization scenarios, pull_latest_from_table_or_query seems more appropriate as we typically want the latest feature values for each entity when populating the online store. However, the materialization logic change in 0.50 suggests there may be use cases requiring pull_all_from_table_or_query.
Investigation needed:
- Review the materialization logic changes between 0.49 and 0.50
- Determine the intended behavior for ClickHouse offline store materialization
- Ensure consistency across different offline store implementations
Activity
leiyangyou commentedon Jul 2, 2025
it's probably only broken in local engine
franciscojavierarceo commentedon Jul 2, 2025
@HaoXuAI
leiyangyou commentedon Jul 2, 2025
it appears that it does a dedupe in the compute engine downstream
haoxu0 commentedon Jul 2, 2025
That's right, some people ask for the feature that pulls all features instead of just the latest feature. But it should be something simple to implement the
pull_all_from_table_or_queryfor ClickHouse.leiyangyou commentedon Jul 2, 2025
i would prefer in some cases to be able to configure this (whether to pull all features then dedupe downstream, or just use pull_latest_from_table_or_query) e.g. when loading from data sources where there are lots of old records
haoxu0 commentedon Jul 8, 2025
That should be doable with some changes. but in the long run we will deprecate the old flow for materialization
franciscojavierarceo commentedon Oct 14, 2025
@HaoXuAI we should update our docs here in this case because I believe we're now inconsistent.
CC @ntkathole @jyejare
astronautas commentedon Oct 15, 2025
Yes please!
For the context, our team uses (and maintains) the Clickhouse contrib offline store. We were a bit taken aback by the fact that under the hood it started using
pull_all_from_table_or_queryinstead of*_latest_*- explicitly marked optional in docs - which both broke Clickhouse contrib from 0.50.0, but also makes me think whether it actually pushes the latest values into an online store e.g. Redis anymore 🤔 We will be soon contributing the adjustments for CH store, but it was quite a surprise.So, basically, in the docs, the method should be marked as "mandatory" instead of optional and it should be clearly stated whether only latest values are fetched with get_online_features anymore.
Also, how about performance implications of pulling all features instead of latest?
franciscojavierarceo commentedon Oct 15, 2025
@astronautas this is really helpful information to know.
Do you have any suggestions on how we can better reconcile this? We don't typically don't want to break contributors/users of different stores so let me know if there's more we can do to raise changes so we can capture these issues before a release. Thanks for enduring some of the pain ❤️ sorry for it!
astronautas commentedon Oct 15, 2025
@franciscojavierarceo No worries at all. Not sure there's an easy answer, but, to start with, one of the integration tests (
test_universal_materialization_consistency) fortest-python-universal-clickhouse-offlinewas failing. How come it slipped into the release? Maybe I am just missing something here, but something does not add up.Just double checked on master, yup, getting this:
make test-python-universal-clickhouse-offline
so probably we don't run this target on master. Suppose one of the suggestions would be to run it? Though it's a contrib lib, not sure how you treat those.
haoxu0 commentedon Oct 16, 2025
Right, not all integration tests are run on the master branch. Sorry for the trouble and really appreciate your contributions!
astronautas commentedon Oct 16, 2025
@HaoXuAI no worries, just a thing to consider for the future!
astronautas commentedon Oct 17, 2025
@franciscojavierarceo Shall we make a release? We're eagerly waiting for this fix 🙏 .
ntkathole commentedon Mar 25, 2026
Issue resolved