@Entity
@Inheritance(strategy=InheritanceType.TABLE_PER_CLASS)
public abstract class LegalParty {
@NaturalId(mutable=true)
@Column(unique=true, nullable=false)
private String id;
}
public class Person extends LegalParty {
}
public class Company extends LegalParty {
}
The NaturalId implementation seems to effectively require that my id column is unique across all implementing classes.
The following code:
Person person = new Person();
person.setId("THE_SAME_AS_THE_OTHER");
session.save(person);
Company company = new Company();
company.setId("THE_SAME_AS_THE_OTHER");
session.save(company);
SimpleNaturalIdLoadAccess<Company> companyLoadAccess = session.bySimpleNaturalId(Company.class);
Company foundCompany = companyLoadAccess.loadOptional("THE_SAME_AS_THE_OTHER").orElseThrow(() -> new IllegalArgumentException());
SimpleNaturalIdLoadAccess<Person> personLoadAccess = session.bySimpleNaturalId(Person.class);
Person foundPerson = personLoadAccess.loadOptional("THE_SAME_AS_THE_OTHER").orElseThrow(() -> new IllegalArgumentException());
The last line throws an IllegalArgumentException as nothing is returned from the loadOptional method call.
Diving into the code, I see that in StatefulPersistenceContext::locateProperPersister() returns: session.getFactory().getMetamodel().entityPersister( persister.getRootEntityName() )
which returns the UnionSubclassEntityPersister for LegalParty, which results in that the hashCode generated for the naturalId as the key value in the naturalIdToPkMap is the same, so there is only one row in this Map.
However, there are two rows in the pkToNaturalIdMap as the EntityKey used by the primary key is generated using the EntityPersister for the subclass (Person or Company).
But both of these rows have the same value for the NaturalId part.
This results in a lookup that returns the primary key for a different object type, so the primary key lookup fails to find anything, even thought both objects exists in the PersistenceContext.
Shouldn’t the UnionSubclassEntityPersister for the proper subclass be used to generate the naturalId hashcode?
Keys must be unique across the hierarchy, otherwise some kind of discriminator should be part of the key. Not sure why you need a table per class inheritance here if you the key is not unique across the tables. If you just want reuse, use a @MappedSuperclass and query every entity type individually by natural id. Thinking about it, what would you like Hibernate to return if you query LegalParty by e.g. PK or NaturalID 1 when there are multiple objects? See the problem? That’s just not how this works. If this is the database design you have to use, you have to decide which object to try to fetch first etc.
Why must keys be unique across the hierarchy, when the same is not a requirement for a regular @id key? I understand the issue when querying by LegalParty, but the same can be said for the same hierarchy and querying by @Id on the root entity class - is that not correct? (assuming that your primary key is generated by two different sequences, allowing for the same primary key in two different implementing classes)
I find it strange and inconsistent that the two behaviors are not the same, and that I am able to model it in this way, and then have a silent error that my persisted object does not exist, when it clearly does. I would find it more correct that I got an exception that told me what the exact problem was, which I imagine I would get if I queried by LegalParty. Or even better that this requirement was enforced when persisting the second object, but I understand the problems implementing this.
I believe that the reason that they were originally modelled with a table_per_class and not single_table, is that we could not be sure, that for all our customers that there would not be a case of an id being the same across two tables. All legal parties have natural id’s, but they are only guaranteed unique within a legal party type, this makes it impossible to use single_table inheritance.
Anyway, I am probably stuck with this design, but I can’t see how a @MappedSuperclass makes a difference.
One last thing, I was unable to find this behavior documented, is this just me looking in the wrong places?
How come you think this does not also apply for primary keys? It has the same “limitations” for the same reasons. The primary key must be unique across the hierarchy, that’s just how this is. If you see that it works, please post an example. I have the feeling that if that works, this would be a bug as that might lead to some internal inconsistencies.
The @MappedSuperclass approach will only help you in the sense that you can keep the Java class hierarchy and reuse the common fields. Every subclass which will be a separate entity then can have its own id then.
I can only refer you to the JPA specification regarding documentation, but I didn’t search for this yet to find a reference for you.
I made an incorrect assumption, when seeing how the EntityKey was generated for an object, which seems to use the EntityPersister of the subclass. I, therefore, assumed that both objects could exist in the persistence context at the same time as they would have different keys. I had missed that the generateHashCode() method in EntityKey calls persister.getRootEntityName() to generate the hash code
I guess I am just surprised that it is possible to persist the entities as described without any errors, and that querying by subclass returns one of the objects, but not the other, and does so without throwing an exception, while querying by the root class will throw an exception.
Would it not be possible in AbstractEntityPersister’s method loadEntityIdByNaturalId after the line: final Object hydratedId = getIdentifierType().hydrate( rs, getIdentifierAliases(), session, null );
to throw an exception if rs.next() returns true?
Combined with a check that a NaturalIdResolutionCache’s two maps pkToNaturalIdMap and naturalIdToPkMap have the same size after being written to?
Otherwise, one is susceptible to silent errors, without checking that an object in another subclass table does not exist with that same Id.
I guess I am just surprised that it is possible to persist the entities as described without any errors, and that querying by subclass returns one of the objects, but not the other, and does so without throwing an exception, while querying by the root class will throw an exception.
Surprised or not, Hibernate can’t check everything for you and if you have bad data, then bad things will happen
Having said that, if you think you know how to improve this, you are free to create a JIRA issue and provide a PR for this along with a test that shows the scenario.
