search

hibernate / hibernate-reactive

A reactive API for Hibernate ORM, supporting non-blocking database drivers and a reactive style of interaction with the database.
https://hibernate.org/reactive
Apache License 2.0
432 stars 90 forks source link

checkNaturalId() #210

Open gavinking opened 4 years ago

gavinking commented 4 years ago

In DefaultReactiveFlushEntityEventListener there is a checkNaturalId() method which seems to exist only to throw an exception if the user changes an immutable, (i.e. non-updatable) natural id property of the entity.

Now there is actually a code path in which this method can wind up hitting the database to retrieve a snapshot in getNaturalIdSnapshot(), which seems a bit excessive to me. (We hit the database to see if the user has an error in their code?)

Anyway the point is that this method would blow up in HR.

Now, I guessing, but I'm not certain, that this method is probably only called in the case of the legacy saveOrUpdate() method, which we don't support in HR, since that's usually where you have no snapshot. (IIRC, we always fetch a snapshot when using merge(), is that right?)

Now, we could either:

  1. write a reactive version of getNaturalIdSnapshot(), or
  2. simply disable this check and delete the whole method, or
  3. just ignore it for now and wait and see if it ever blows up for anyone in order to figure out what cases it occurs in.

Since option 3 means doing no work, it sounds like a pretty excellent choice.

DavideD commented 4 years ago

Option 3 sounds good to me

gavinking commented 4 years ago

@sebersole could you clarify for me in exactly which situations EntityPersister.getNaturalIdentifierSnapshot() can be called?

For us to reactify the whole code path that leads into this method looks like quite a lot of work. It would be nice to know how important this method is.