Core: Fix StructLikeWrapper.equals exception with comparator failure#15945
Merged
huaxingao merged 1 commit intoapache:mainfrom Apr 16, 2026
Merged
Core: Fix StructLikeWrapper.equals exception with comparator failure#15945huaxingao merged 1 commit intoapache:mainfrom
huaxingao merged 1 commit intoapache:mainfrom
Conversation
krvikash
approved these changes
Apr 12, 2026
anoopj
reviewed
Apr 12, 2026
| return comparator.compare(this.struct, that.struct) == 0; | ||
| try { | ||
| return comparator.compare(this.struct, that.struct) == 0; | ||
| } catch (Throwable e) { |
Contributor
There was a problem hiding this comment.
Throwable is a bit too broad, Can you just catch the correct exception (which is IllegalArgumentException?) At worst, catch only Exception
Contributor
There was a problem hiding this comment.
+1, worst case we can just have RTE catched
Member
Author
There was a problem hiding this comment.
I'm wondering if catching only IllegalArgumentException is really the right approach. Because the error originates from StructLike#get, the behavior depends on each implementation and the specific exception type is not guaranteed. I've changed this to catch RuntimeException instead.
fac9d88 to
5dc2870
Compare
anoopj
approved these changes
Apr 13, 2026
nastra
reviewed
Apr 13, 2026
nastra
approved these changes
Apr 14, 2026
huaxingao
approved these changes
Apr 16, 2026
Contributor
|
Thanks @ebyhr for the PR! Thanks @anoopj @krvikash @nastra @singhpk234 for the review! |
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.
StructLikeWrapper.equals threw an exception when the comparator threw an exception. This PR wraps the comparator.compare call in a try-catch so that type mismatches return false instead of propagating the exception.
The newly added test fails if we revert the StructLikeWrapper.equals change: