Skip to content

fix: Return False from __eq__ on cross-type comparison - #6637

Merged
ntkathole merged 3 commits into
feast-dev:masterfrom
larrysingleton007:fix/eq-cross-type-comparison
Jul 30, 2026
Merged

fix: Return False from __eq__ on cross-type comparison#6637
ntkathole merged 3 commits into
feast-dev:masterfrom
larrysingleton007:fix/eq-cross-type-comparison

Conversation

@larrysingleton007

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Every registry object's __eq__ raised TypeError when compared against an object of a different type:

if not isinstance(other, X):
    raise TypeError("Comparisons should only involve X class objects.")

Comparing objects of different types should be False, not an error. Because the apply/diff path compares registered objects against incoming ones, this meant changing a feature view's data source type and re-running feast apply over an existing registry crashed with TypeError: Comparisons should only involve <X> class objects.

This replaces the raise with return False across all 35 affected __eq__ methods (DataSource and its Kafka/Request/Kinesis subclasses, the offline and contrib sources, Entity, FeatureView, BaseFeatureView, OnDemandFeatureView, StreamFeatureView, FeatureService, SavedDataset, Project, ProjectMetadata, the transformations, Aggregation, LabelView, Permission, and the policies). PushSource.__eq__ already handled a cross-type comparison this way, so this makes every other __eq__ consistent with it rather than introducing a new pattern (there is no return NotImplemented anywhere in the codebase).

Which issue(s) this PR fixes:

Fixes #6636

Checks

  • I've made sure the tests are passing.
  • My commits are signed off (git commit -s)
  • My PR title follows conventional commits format

Testing Strategy

  • Unit tests
  • Integration tests
  • Manual tests
  • Testing is not required for this change

Added cross-type regression tests asserting False (not a raise) for DataSource, Entity, and a policy object, and updated test_label_view.py::test_equality_type_check, which previously asserted the removed TypeError. Full unit suite passes locally.

Misc

Release note: Comparing two Feast objects of different types now returns False instead of raising TypeError, so changing a feature view's data source type and re-applying no longer crashes.

Scope: this sweeps all 35 sites across the object model. Happy to narrow to just the DataSource hierarchy (where the crash reproduces), with a follow-up for the rest, if a smaller diff is preferred.

As an outside contributor I can't apply labels, so a maintainer will need to add a kind/bug label and run ok-to-test.

@larrysingleton007
larrysingleton007 requested review from a team and sudohainguyen as code owners July 23, 2026 15:26
@codecov-commenter

codecov-commenter commented Jul 23, 2026

Copy link
Copy Markdown

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

❌ Patch coverage is 59.09091% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 46.35%. Comparing base (adb8c1c) to head (d2fc30b).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
sdk/python/feast/base_feature_view.py 0.00% 1 Missing ⚠️
sdk/python/feast/data_source.py 75.00% 1 Missing ⚠️
...ores/contrib/athena_offline_store/athena_source.py 0.00% 1 Missing ⚠️
...ontrib/couchbase_offline_store/couchbase_source.py 0.00% 1 Missing ⚠️
.../contrib/mssql_offline_store/mssqlserver_source.py 0.00% 1 Missing ⚠️
...ores/contrib/oracle_offline_store/oracle_source.py 0.00% 1 Missing ⚠️
.../contrib/postgres_offline_store/postgres_source.py 0.00% 1 Missing ⚠️
...ine_stores/contrib/ray_offline_store/ray_source.py 0.00% 1 Missing ⚠️
...stores/contrib/spark_offline_store/spark_source.py 66.66% 0 Missing and 1 partial ⚠️
...stores/contrib/trino_offline_store/trino_source.py 0.00% 1 Missing ⚠️
... and 8 more
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6637      +/-   ##
==========================================
+ Coverage   45.98%   46.35%   +0.36%     
==========================================
  Files         414      414              
  Lines       50037    50052      +15     
  Branches     7147     7151       +4     
==========================================
+ Hits        23012    23201     +189     
+ Misses      25413    25229     -184     
- Partials     1612     1622      +10     
Flag Coverage Δ
go-feature-server 30.58% <ø> (+0.05%) ⬆️
python-unit 47.65% <59.09%> (+0.38%) ⬆️
Files with missing lines Coverage Δ
sdk/python/feast/aggregation/__init__.py 83.33% <100.00%> (+3.33%) ⬆️
sdk/python/feast/data_format.py 84.37% <100.00%> (+1.04%) ⬆️
sdk/python/feast/entity.py 88.05% <100.00%> (+2.98%) ⬆️
sdk/python/feast/feature.py 73.33% <100.00%> (+3.56%) ⬆️
sdk/python/feast/feature_service.py 82.27% <100.00%> (+1.26%) ⬆️
sdk/python/feast/feature_view.py 87.01% <100.00%> (+1.29%) ⬆️
...thon/feast/infra/offline_stores/bigquery_source.py 61.34% <100.00%> (+1.68%) ⬆️
...k/python/feast/infra/offline_stores/file_source.py 71.42% <100.00%> (+1.24%) ⬆️
...thon/feast/infra/offline_stores/redshift_source.py 62.93% <100.00%> (+1.39%) ⬆️
...hon/feast/infra/offline_stores/snowflake_source.py 56.95% <100.00%> (+1.32%) ⬆️
... and 22 more

... and 7 files with indirect coverage changes


Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update abffebc...d2fc30b. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@larrysingleton007

Copy link
Copy Markdown
Contributor Author

@sudohainguyen - awaiting your review please

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes Feast registry-object equality semantics by making cross-type __eq__ comparisons return False instead of raising TypeError, preventing feast apply crashes when an object’s concrete type changes (e.g., switching a feature view’s data source type) and the registry diff logic compares old vs new objects.

Changes:

  • Replaced cross-type __eq__ TypeError raises with return False across the affected core object model classes (including data sources, feature views/services, projects, transformations, labeling, permissions).
  • Updated and added unit tests to assert cross-type comparisons return False (and that != remains the inverse), including updating a prior test that expected TypeError.
  • Added a regression test covering the original repro scenario for data source type changes.

Reviewed changes

Copilot reviewed 34 out of 34 changed files in this pull request and generated no comments.

Show a summary per file
File Description
sdk/python/tests/unit/test_label_view.py Updates LabelView equality test to assert cross-type comparisons return False.
sdk/python/tests/unit/test_eq_cross_type.py Adds a parameterized regression suite asserting cross-type equality returns False for multiple core types.
sdk/python/tests/unit/test_entity.py Adds regression test for Entity cross-type equality returning False.
sdk/python/tests/unit/test_data_sources.py Adds regression test covering cross-type equality for DataSource subclasses (e.g., FileSource vs SnowflakeSource).
sdk/python/tests/unit/permissions/test_policy.py Adds regression test for policy cross-type equality returning False.
sdk/python/feast/transformation/substrait_transformation.py Makes SubstraitTransformation __eq__ return False on cross-type comparisons.
sdk/python/feast/transformation/ray_transformation.py Makes RayTransformation __eq__ return False on cross-type comparisons.
sdk/python/feast/transformation/python_transformation.py Makes PythonTransformation __eq__ return False on cross-type comparisons.
sdk/python/feast/transformation/pandas_transformation.py Makes PandasTransformation __eq__ return False on cross-type comparisons.
sdk/python/feast/stream_feature_view.py Makes StreamFeatureView __eq__ return False on cross-type comparisons.
sdk/python/feast/saved_dataset.py Makes SavedDataset __eq__ return False on cross-type comparisons.
sdk/python/feast/project.py Makes Project __eq__ return False on cross-type comparisons.
sdk/python/feast/project_metadata.py Makes ProjectMetadata __eq__ return False on cross-type comparisons.
sdk/python/feast/permissions/policy.py Makes Policy implementations’ __eq__ return False on cross-type comparisons.
sdk/python/feast/permissions/permission.py Makes Permission __eq__ return False on cross-type comparisons.
sdk/python/feast/on_demand_feature_view.py Makes OnDemandFeatureView __eq__ return False on cross-type comparisons.
sdk/python/feast/labeling/label_view.py Makes LabelView __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/snowflake_source.py Makes SnowflakeSource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/redshift_source.py Makes RedshiftSource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/file_source.py Makes FileSource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/contrib/trino_offline_store/trino_source.py Makes TrinoSource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/contrib/ray_offline_store/ray_source.py Makes RaySource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/contrib/postgres_offline_store/postgres_source.py Makes PostgreSQLSource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/contrib/oracle_offline_store/oracle_source.py Makes OracleSource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/contrib/mssql_offline_store/mssqlserver_source.py Makes MsSqlServerSource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/contrib/couchbase_offline_store/couchbase_source.py Makes CouchbaseColumnarSource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/contrib/athena_offline_store/athena_source.py Makes AthenaSource __eq__ return False on cross-type comparisons.
sdk/python/feast/infra/offline_stores/bigquery_source.py Makes BigQuerySource __eq__ return False on cross-type comparisons.
sdk/python/feast/feature_view.py Makes FeatureView __eq__ return False on cross-type comparisons.
sdk/python/feast/feature_service.py Makes FeatureService __eq__ return False on cross-type comparisons.
sdk/python/feast/entity.py Makes Entity __eq__ return False on cross-type comparisons.
sdk/python/feast/data_source.py Makes DataSource and streaming/request source __eq__ return False on cross-type comparisons.
sdk/python/feast/base_feature_view.py Makes BaseFeatureView __eq__ return False on cross-type comparisons.
sdk/python/feast/aggregation/init.py Makes Aggregation __eq__ return False on cross-type comparisons.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@larrysingleton007

Copy link
Copy Markdown
Contributor Author

@ntkathole could you take a look at this one or reassign? The auto-assigned reviewer doesn't seem to be active lately, so it's been sitting since the 23rd. Checks are green and there are no open review threads.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 34 out of 34 changed files in this pull request and generated no new comments.

@larrysingleton007

Copy link
Copy Markdown
Contributor Author

@ntkathole @franciscojavierarceo @jyejare @aniketpalu sorry for the broad ping, but this one still has no active reviewer. The auto-assigned reviewer hasn't reviewed anything here since October, and a second user just confirmed hitting the same bug over on #6636 (swapping a FeatureView source from FileSource to SparkSource). I've since extended the fix to cover their case as well. Checks are green, could one of you pick it up or assign someone?

@franciscojavierarceo franciscojavierarceo left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cross-type guards are applied consistently across the affected domain objects, preserve same-type comparison behavior, and include broad regression coverage. I found no blocking issue in the remote diff.

Every registry object's __eq__ did
`if not isinstance(other, X): raise TypeError("Comparisons should only involve X class objects.")`.
Raising from __eq__ on an operand of a different type is wrong: comparing
objects of different types should be False, not an error. As a result, changing
a feature view's data source type and re-applying over an existing registry
crashed with `TypeError: Comparisons should only involve <X> class objects.`

Replace the raise with `return False` across all 35 affected __eq__ methods,
matching PushSource.__eq__ which already handles a cross-type comparison this
way. Update the LabelView equality test (it asserted the removed TypeError) and
add cross-type regression tests for DataSource, Entity, and policy objects, all
asserting False rather than a raise.

Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com>
…odel

Assert that cross-type comparison returns False (never raises) for the
importable core types touched by the sweep: Entity, Project, Aggregation,
Permission, all four policies, the Kafka/Kinesis/Request and
File/BigQuery/Redshift/Snowflake sources, FeatureView, and FeatureService.
Complements the existing per-type tests (DataSource, Entity, LabelView,
RoleBasedPolicy) and raises patch coverage on the sweep. Contrib sources and
optional-dependency transformations are omitted since the unit env can't
import them.

Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com>
…-type comparison

SparkSource.__eq__ passed the shared DataSource base check for any
DataSource subclass and then accessed spark-only attributes, so comparing
against a FileSource with a matching name raised AttributeError instead
of returning False (reported on feast-dev#6636 when swapping a FeatureView's
source from FileSource to SparkSource). Feature.__eq__ and the
FileFormat/StreamFormat __eq__ accessed attributes of the other operand
unguarded and failed the same way; they raised AttributeError rather
than the TypeError pattern, which is why the original sweep missed them.

Signed-off-by: Larry Singleton <166439969+larrysingleton007@users.noreply.github.com>
@ntkathole
ntkathole force-pushed the fix/eq-cross-type-comparison branch from 1af5c88 to d2fc30b Compare July 30, 2026 06:09
@ntkathole
ntkathole merged commit 0f149a9 into feast-dev:master Jul 30, 2026
17 of 22 checks passed
@larrysingleton007
larrysingleton007 deleted the fix/eq-cross-type-comparison branch July 30, 2026 13:23
franciscojavierarceo pushed a commit that referenced this pull request Aug 21, 2026
# [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))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

__eq__ raises TypeError on cross-type comparison instead of returning NotImplemented

5 participants