5:40 a.m.
Hi,
When HHH-12054 was fixed (see
https://github.com/hibernate/hibernate-orm/pull/2042/files), we took into
account the fact that original and target could be set to
LazyPropertyInitializer.UNFETCHED_PROPERTY in the
AbstractStandardBasicType#getReplacement() method whereas we haven't
changed the prototype of the method.
Meaning we expect for instance LazyPropertyInitializer.UNFETCHED_PROPERTY
to be a T, which is not the case.
This led to ClassCastException such as in
https://hibernate.atlassian.net/browse/HHH-12555 . To be honest, I'm
wondering why we don't have more of them.
We have 2 ways of fixing this issue:
1- change the getReplacement() prototype to take Object parameters and
return an Object as the originating replace() method
2- move the LazyPropertyInitializer.UNFETCHED_PROPERTY logic up to
replace() and make getReplacement() a "I'm sure everything is OK, now get
me a clone of my original object" thing.
1- will obviously break compatibility with any user type extending
AbstractStandardBasicType#getReplacement(). 2- will slightly change the
meaning of replace() and getReplacement() and might lead to subtle behavior
changes in user applications.
2- also means that we would have to override both replace() and
getReplacement() in BlobType and allegates.
I pushed a commit for 1- at
https://github.com/gsmet/hibernate-orm/commit/73e66e40f3438794375451145af...
so that you can fully grasp the issue.
Not saying it's the path we should take.
Thoughts?
--
Guillaume
4:02 a.m.
Any thoughts on this one? Luis, I think your opinion would help.
replace() being currently final, we cannot override it in child classes so
I tried another approach trying to avoid breaking what has been exposed to
the users:
https://github.com/hibernate/hibernate-orm/pull/2390
It's not pretty but I think it's the best we can do.
I still have a corner case when I don't know exactly what to do: what if
the original blob is null and the target blob is unfetched: should we set
the blob to null or initialize a new empty blob?
The current situation is:
1/ if original is null and target is null, it returns null
2/ if original is null but target is not null, it returns an empty blob
So the question is what should we do if original is null and target is
unfetched? Should we do 1/ or 2/?
--
Guillaume
3:18 p.m.
Hi Guillaume,
Thanks for reminding me about this in my PR for HHH-12054 for backporting
to 5.1.
I'm planning to take a look at this, hopefully early next week.
Regards,
Gail
On Wed, Jul 4, 2018 at 2:02 AM, Guillaume Smet <guillaume.smet(a)gmail.com>
wrote:
Any thoughts on this one? Luis, I think your opinion would help.
replace() being currently final, we cannot override it in child classes so
I tried another approach trying to avoid breaking what has been exposed to
the users:
https://github.com/hibernate/hibernate-orm/pull/2390
It's not pretty but I think it's the best we can do.
I still have a corner case when I don't know exactly what to do: what if
the original blob is null and the target blob is unfetched: should we set
the blob to null or initialize a new empty blob?
The current situation is:
1/ if original is null and target is null, it returns null
2/ if original is null but target is not null, it returns an empty blob
So the question is what should we do if original is null and target is
unfetched? Should we do 1/ or 2/?
--
Guillaume
_______________________________________________
hibernate-dev mailing list
hibernate-dev(a)lists.jboss.org
https://lists.jboss.org/mailman/listinfo/hibernate-dev
1:33 p.m.
Hi Gail,
Don't waste your time on it for now. We had an interesting discussion with
Yoann about it and I plan to revisit it.
I'll ping you when it's done.
Thanks.
--
Guillaume
On Thu, Jul 5, 2018 at 10:18 PM Gail Badner <gbadner(a)redhat.com> wrote:
Hi Guillaume,
Thanks for reminding me about this in my PR for HHH-12054 for backporting
to 5.1.
I'm planning to take a look at this, hopefully early next week.
Regards,
Gail
On Wed, Jul 4, 2018 at 2:02 AM, Guillaume Smet <guillaume.smet(a)gmail.com>
wrote:
> Any thoughts on this one? Luis, I think your opinion would help.
>
> replace() being currently final, we cannot override it in child classes so
> I tried another approach trying to avoid breaking what has been exposed to
> the users:
> https://github.com/hibernate/hibernate-orm/pull/2390
>
> It's not pretty but I think it's the best we can do.
>
> I still have a corner case when I don't know exactly what to do: what if
> the original blob is null and the target blob is unfetched: should we set
> the blob to null or initialize a new empty blob?
>
> The current situation is:
> 1/ if original is null and target is null, it returns null
> 2/ if original is null but target is not null, it returns an empty blob
>
> So the question is what should we do if original is null and target is
> unfetched? Should we do 1/ or 2/?
>
> --
> Guillaume
> _______________________________________________
> hibernate-dev mailing list
> hibernate-dev(a)lists.jboss.org
> https://lists.jboss.org/mailman/listinfo/hibernate-dev
>
2357
days inactive
2399
days old
3 comments
2 participants
participants (2)
-
Gail Badner -
Guillaume Smet