Skip to content

fix: make ToJson PartialEq<dyn Any> consistent with PartialEq#4446

Merged
andygrove merged 2 commits into
apache:mainfrom
0lai0:fix-4430-tojson-partialeq
May 27, 2026
Merged

fix: make ToJson PartialEq<dyn Any> consistent with PartialEq#4446
andygrove merged 2 commits into
apache:mainfrom
0lai0:fix-4430-tojson-partialeq

Conversation

@0lai0
Copy link
Copy Markdown
Contributor

@0lai0 0lai0 commented May 27, 2026

Which issue does this PR close?

Closes #4430

Rationale for this change

PartialEq<dyn Any> is the path DataFusion uses to compare Arc<dyn PhysicalExpr> values in the physical planner. The previous impl only compared expr and timezone, so two ToJson expressions that differed only in ignore_null_fields (or historically, timezone) would compare equal through that path, risking incorrect deduplication or caching.

What changes are included in this PR?

Delegate PartialEq<dyn Any> to self == other, which calls the regular PartialEq impl and ensures both paths always compare the same fields.

How are these changes tested?

Three new unit tests in json_funcs::to_json::test:

  • test_partial_eq_same identical exprs are equal via both paths
  • test_partial_eq_dyn_any_differs_on_timezone different timezone is not equal via dyn Any
  • test_partial_eq_dyn_any_differs_on_ignore_null_fields different ignore_null_fields is not equal via dyn Any

@andygrove
Copy link
Copy Markdown
Member

Thanks @0lai0 . Could you fix linter errors?

Copy link
Copy Markdown
Member

@andygrove andygrove left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @0lai0

@andygrove andygrove merged commit aa91ab3 into apache:main May 27, 2026
62 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ToJson PartialEq<dyn Any> impl is inconsistent with PartialEq impl

2 participants