null collections

[gavinking]

It has come to my attention that the spec does not assign any interpretation to a null collection in an entity. That's not good at all, and I think we should fix that (or at least call it out as being explicitly undefined).

Hibernate treats a null collection as equivalent to an empty collection.

@anija-anil What does EclipseLink do?

[busitech]

We suggest that the spec define a null value on a collection property to mean that no action can be taken by the persistence layer based on the value. Only an empty collection object should represent an empty collection.

Hibernate's long standing behavior of assigning unwanted meaning to null values on collection properties has been a great source of problems in a wide variety of scenarios.

We also suggest that the spec define a return value for persist, to be consistent with merge on EntityManager, and further define that a lazy collection property having a null value on an object passed to persist never be null on the object returned, but replaced with a proxy that will discover the true contents of the collection (empty or not) at the next time it is accessed.

This would allow a lazy collection property on an object just persisted to be lazy-fetched later and contain accurate results, assuming that related entities have also been persisted.

[gavinking]

@busitech such a change would break thousands upon thousands of existing programs.

[anija-anil]

@gavinking
Based on my analysis, EclipseLink does not automatically turn null collection into empty collections. Instead, collections are allowed to remain null until they are actually needed. When required during runtime operations, EclipseLink creates empty collection containers on demand. Throughout the persistence lifecycle, null values are handled explicitly rather than being implicitly treated as empty.

[gavinking]

@anija-anil OK, if I understood you, that sounds like the same thing Hibernate does. When I say "treats a null collection as equivalent to an empty collection", I mean that if I persist() an entity with a null collection, and then load the same entity using find(), I will get an entity with an empty collection back.

[hantsy]

When I tried to migrate my Cargotracker fork to Jakarta EE 11 and transformed some embeddable classes to use record, some testing code broke. I will append the certain problem when I move on to it later.

Eclipse Cargotracker itself used a null object pattern to set up the null collections and null object association when initializing an entity.

[gavinking]

I have no idea what a "null object pattern" is.

[busitech]

The opening premise of this issue — that the spec does not assign an interpretation to a null collection — is correct, and worth addressing. The resolution should be principled, because current implementation behavior demonstrates that this vacuum has been filled with different answers, and not all of them are correct.

I ran a small empirical test against Hibernate ORM 7.3.1.Final and EclipseLink 5.0.0 to confirm current behavior before commenting. Three findings are material.

First, on merge of a detached entity with a null lazy @OneToMany, Hibernate installs an uninitialized PersistentBag on the returned managed entity. Accessing the collection triggers a SELECT and returns the correct contents from the database. This is correct behavior, and it is what I am proposing the spec require in general. For the same scenario, EclipseLink returns the managed entity with an initialized, empty IndirectList; the pre-existing child in the database is invisible to the managed entity.

Second, on persist of a new Owner with null children followed by persist of three Child entities whose @ManyToOne points back to the owner, neither provider's managed entity reflects the freshly-inserted children after flush. Hibernate leaves owner.children as null (traversal throws NullPointerException). EclipseLink leaves owner.children as an initialized empty collection of size 0 (traversal silently returns zero members). In both cases, a fresh find() in a new session correctly shows all three children, confirming the database is correct and the issue lies entirely in the managed-entity state produced by persist.

Third, the above is not a theoretical concern. I extended the test with Hibernate Search 8.3.0.Final using its stock in-memory Lucene backend with synchronous indexing. The owner is @Indexed with its children included via @IndexedEmbedded. After the persist sequence above and transaction commit, the Lucene index contains the owner but reports zero matches on a query that should match one of the just-inserted children through the embedded index field. Running the stock MassIndexer against the same database afterward, which reloads the owner in a fresh session, produces a correct index. The flush-time automatic indexing path produces an incorrect document; the fresh-load path produces a correct one. The difference is entirely the state of the managed entity's collection-valued property.

These observations motivate the following two principles for JPA 4.0.0:

Principle 1. A null value on a collection-valued property carries no information. The persistence provider must not infer domain state from it and must not take action based on it. Only an empty collection represents the domain assertion that the collection is empty.

Principle 2. When a caller-supplied null on a collection-valued property is processed by persist or merge, the managed entity's post-condition for that property must be an uninitialized proxy that will load its contents from the database on first access. null must never be a valid post-condition of a collection-valued property on a managed entity, and a null input must not be silently replaced with an eagerly-initialized empty collection, which commits the entity to a domain assertion the caller did not make and renders flush-time traversal incorrect.

These principles separate concerns that the current behavior conflates: domain state (whether a relationship has members), persistence state (whether that information has been fetched or provided), and absence of information (the caller has said nothing). Hibernate already applies these principles on merge; the spec should require the same on persist, and should specify what both providers must produce when null collection inputs are encountered.

Secondary proposal. EntityManager.persist should return the managed entity, matching the contract of merge. This is an independent consistency improvement and does not depend on the resolution of the two principles above.

The spec does not currently assign a defined interpretation to this case. The implementations have filled the vacuum in incompatible and individually problematic ways, and in the case of Hibernate Search, the current ORM behavior propagates to produce a corrupted search index from a straightforward usage pattern. JPA 4.0.0 is the right place to resolve this.

[gavinking]

@busitech

Consider:

@Entity 
class Thing {
    @GeneratedValue @Id 
    Long id;

    @ElementCollection 
    List<String> labels;
}

A null value on a collection-valued property carries no information.

This would break thousands of programs.

We would have to suddenly start rejecting the following until now perfectly-legal and perfectly-reasonable code:

em.persist(new Thing());

So your proposal is a non-starter.

[gavinking]

First, on merge of a detached entity with a null lazy @OneToMany, Hibernate installs an uninitialized PersistentBag on the returned managed entity. Accessing the collection triggers a SELECT and returns the correct contents from the database.

This isn't actually quite correct. If you test this more carefully, you'll see that what happens here is that the object returned by merge() does indeed have an uninitialized collection, but that uninitialized collection has a queued Clear operation, so that that when the collection is subsequently accessed, it's contents are cleared out, reflecting the fact that a null collection was previously merged to it. This behavior is completely consistent with what I previously stated that HIbernate always treats null collections as empty collections.

[busitech]

My initial comment specified a lazy @OneToMany (the inverse-side) case. Merging null there does not enqueue a Clear; the pre-existing children survive. This is why your previous statement of "always treats" is false. I tested this on Hibernate and EclipseLink, with and without class enhancement, and all four scenarios agree.

Your example is the owning-side case. This is not the case I raised, but the principles apply equally well.

Closing a harmful loophole created by an ambiguity the spec has never addressed does impose the need to do some refactoring, but that refactoring is neither difficult nor unexpected when upgrading to a new release. Defining what was previously undefined, in a way that promotes good design, accuracy and interoperability, and prevents unwanted side-effects is excellent and necessary.

When the in-memory object graph disagrees with the database state, the persistence provider has failed at its core responsibility. Routines that traverse the managed graph (such as Hibernate Search) are then silently wrong. This is what our business application and tests reveal.

That any persistence provider assigned meaning to a collection property left null is unhelpful, because it overstates what the caller asserted. This pattern, when taken to its logical conclusion, yields the inability of the persistence layer to fulfill its primary obligation. Instead, it was glossed over out of convenience.

It is not unreasonable to require callers to initialize a collection type property with a meaningful value. It is unreasonable to enshrine a bad design encouraged by an undefined behavior in the absence of a superior design.

As a reminder, this discussion is about improving the next release of the specification. It is not about any particular implementation of the previous release.

[gavinking]

My initial comment specified a lazy @onetomany (the inverse-side) case. Merging null there does not enqueue a Clear;

It absolutely does. Proof is here: https://github.com/hibernate/hibernate-orm/blob/main/hibernate-core/src/test/java/org/hibernate/orm/test/merge/UnownedCollectionMergeTest.java

It is unreasonable to enshrine a bad design encouraged by an undefined behavior in the absence of a superior design.

There is nothing "bad" about the design that both Hibernate and EclipseLink arrived at completely independently. In fact, it's by far the most natural behavior.

Closing a harmful loophole created by an ambiguity the spec has never addressed does impose the need to do some refactoring, but that refactoring is neither difficult nor unexpected when upgrading to a new release.

I agree that the ambiguity in the spec should be resolved. But it should be resolved by simply blessing the current behavior of the existing implementations of Jakarta Persistence. We should not to break two decades worth of working software because of a complaint from one single user. To address your needs, I already added an SPI to Hibernate that lets you obtain the behavior you prefer.