Fix #4629: Handle conflicting creatorProp + ignore conflict (Kotlin data class compatibility) #5439
Conversation
…ignored when deserializing Records
…ignored when deserializing Records
…(Kotlin data class compatibility)
| SettableBeanProperty creatorProp = creator.findCreatorProperty(propName); | ||
|
|
||
| int ix = _propNameMatcher.matchName(propName); | ||
| SettableBeanProperty regularProp = (ix >= 0) ? _propsByIndex[ix] : null; |
There was a problem hiding this comment.
Ok so my problem here is the double lookups in every case, even if in most cases single lookup (for CreatorProperty) should be necessary.
Now, in theory, _propNameMatcher lookup should be enough (matcher should be superset of ALL SettableBeanProperty entries, and type of property could be checked separately, as needed.
I am trying concurrent changes to do that, but having some problems. Alternatively this PR could be refactored to limit need for extra lookups I think.
There was a problem hiding this comment.
Thank you for the answer :)
I’ll review that part of the code as well and try refactoring it.
Once I make the improvements, I’ll update the PR.
|
@JacksonJang Can we actually reproduce the issue with plain Java? It'd be very useful to have test here instead of requiring Kotlin module's test. |
|
Okay. |
|
One thing I noticed is that in Java |
|
@JacksonJang See #5457 which I think is reproduction. |
|
Ok: I think the ideal solution would actually be to handle "split" property case in |
|
Thank you for writing the test case. |
|
@JacksonJang Ok. As I suggested, it might need to be solved elsewhere. I am having Claude check this out as well. And it seems to zero in on |
|
Oh.. sorry for the confusion |
|
@cowtowncoder But I managed to minimize the double lookups in I also added the test case( |
|
|
||
| SettableBeanProperty prop = creator.findCreatorProperty(propName); | ||
| boolean isCreatorProp = (prop != null); | ||
| int ix = _propNameMatcher.matchName(propName); |
There was a problem hiding this comment.
This is the actual lookup from hash structure so access is still duplicated unfortunately (getting property. But I have possible alternative solution, see #5457 .
There was a problem hiding this comment.
Hmmh. Claude didn't actually verify the solution... need to iterate.
There was a problem hiding this comment.
That's unfortunate..
I’ll try combining #5457 with the current code and work on it.
|
The main challenge really is how to avoid automatic name-based ignoral: what should happen is that "split" property handling would ignore |
|
Thank you for the explanation. I understand now that the key is to prevent the automatic name-based ignoral. I’ll do my best to improve the logic accordingly. |
|
Resolved via different PR, closing. |
|
Would it be alright to open a new PR with the latest revision? link : 796281f |
|
Sure!
…On Tue, Dec 2, 2025 at 8:49 PM Jackson ***@***.***> wrote:
*JacksonJang* left a comment (FasterXML/jackson-databind#5439)
<#5439 (comment)>
@cowtowncoder <https://github.com/cowtowncoder>
Would it be alright to open a new PR with the latest revision?
It’s a minor change, but I refactored the code so that the unnecessary
SettableBeanProperty handling is now streamlined under a single prop
variable.
link : 796281f
<796281f>
—
Reply to this email directly, view it on GitHub
<#5439 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAANOGMX5BORJLRT4W3G2TL37ZTVRAVCNFSM6AAAAACNKRSEP2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTMMBVGA3TGOBTGM>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Thanks :) |
2 participants
I resolved #5433