[Develop-KIM]

A null element of an inverse @OneToMany list mapped with @OrderColumn has no row of its own, but it still occupies a list position, so the order column around it should leave a gap — ListInitializer#readCollectionRow pads that gap back into a null when the collection is loaded. Neither ActionQueue does that today, and they fail differently.

Graph-based queue (default since 8.0) — it throws rather than compacting:

org.hibernate.property.access.spi.PropertyAccessException: Error accessing field
[java.lang.Long ListChild.id] by reflection for persistent property [ListChild#id] : null
	at org.hibernate.action.queue.internal.decompose.collection.AbstractOneToManyDecomposer.applyWriteIndexRestrictions(AbstractOneToManyDecomposer.java:1059)
Caused by: java.lang.NullPointerException

decomposeRecreate plans a write-index operation for every entry, so applyWriteIndexRestrictions ends up reading the element identifier off a null entry. The queued-additions path a bit further down in the same class already guards with entry != null && collection.entryExists( entry, ... ), so I used the same guard there. entryCount is already incremented outside the check, so the gap falls out of it.

Legacy queue (hibernate.flush.queue.type=legacy) — no exception, but the index is written compacted, which is what HHH-20658 reports. WriteIndexCoordinatorStandard#writeIndex increments nextIndex inside the null check, so [a, null, b] is stored with indices 0,1 instead of 0,2 and reloads as [a, b]. OneToManyPersister#writeIndex incremented it for every entry through 6.1.7 and the increment moved inside the check in 6.2, so I moved it back out.

Tests cover both queue types (InverseListNullElementTest, InverseListNullElementLegacyTest), asserting the order column holds 0,2 and that the list round-trips as [a, null, b]. Both fail on main and pass with the fix; :hibernate-core:test is green on H2 (17330 tests).

ListElementNullBasicTest still passes — this doesn't change how owned @ElementCollection lists drop nulls.

I used Claude Code to help write this; I reviewed the change and verified it locally.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license
and can be relicensed under the terms of the LGPL v2.1 license in the future at the maintainers' discretion.
For more information on licensing, please check here.

---

---

Please make sure that the following tasks are completed:
Tasks specific to HHH-20658 (Improvement):

• Add tests for feature/improvement
• Update documentation as relevant: javadoc for changed API, documentation/src/main/asciidoc/userguide for all features, documentation/src/main/asciidoc/introduction for main features, links from existing documentation
• Add entries as relevant to migration-guide.adoc (breaking changes) and whats-new.adoc (new features/improvements)

https://hibernate.atlassian.net/browse/HHH-20658

[mbellade]

Thank you @Develop-KIM, the fix looks minimal and correct and would align with the behavior of owned @OneToMany associations, as well as with the read-side expectations.

The JPA spec says:

The persistence provider must maintain a contiguous (non-sparse) ordering

But it doesn't explicitly define the behavior with null values. Furthermore, for unowned lists it says:

Use of the OrderColumn annotation on the unowned side of a relationship is not portable between persistence providers

So it is unfortunate this has been the behavior since 6.2, but I think this change would make sense, though I would keep it in 8.0 and not back-port it to earlier versions. @sebersole wdyt?

[Develop-KIM]

@mbellade @sebersole gentle ping on this one — it's been sitting since the review in July.

Nothing has changed on my side: the branch is still on main, so it lands in 8.0 only as you suggested, and CI is green. Happy to adjust it if the conclusion is that unowned indexed lists should keep the current behaviour instead.

[mbellade]

Sorry for the long wait @Develop-KIM, we try to address all incoming PRs in a timely manner but we're a small team so it's not always easy.

I left a comment for now for a scenario that probably needs further testing, but I'd still like @sebersole to take a look at the direction of this fix and confirm if the new behavior is in line with expectations - in the meantime I've updated the Jira to classify it as an "improvement" instead of a bug.

[mbellade]

We should probably do something similar in decomponseUpdate: you only changed this behavior in the re-create path, but what if we're just updating an ordered List which now contains null entries?

Please add tests for this scenarios, we should verify that a list's update works even when pre-existing null entries were present and/or when new null elements are "added".

[Develop-KIM]

You were right, and it turned out to break in two different ways depending on whether the collection is initialized.

If it isn't, the addition is queued and decomposeQueuedOperations numbers the queued entries by counting only the ones it writes, so a queued null takes up no position: starting from [a, b] and adding null then a child wrote positions 0,1,2 — the new child landed on the gap. The position now comes from where the addition sits in the queue rather than from a counter that only advances on a write.

If it is initialized, it never got that far. PersistentList#computeEntityListChangeSet reports a null slot as an addition of null, and that reaches applyWriteIndexRestrictions, which asks the identifier mapping for the id of a null element — PropertyAccessException on both of the scenarios you described. A null element adds no row, so it isn't reported as an addition any more, and applyUpdateChanges now skips entries that don't exist the same way decomposeRecreate does.

I split the tests along that seam, since the two paths are easy to conflate: a pre-existing null and a newly added one, each once through the queued path and once with the collection read first, all under both ActionQueue implementations. The two initialized cases fail on the previous commit with the exception above, and the queued ones with a wrong order column.

Full hibernate-core suite: 17333 tests, one failure in LockTest#testFindWithPessimisticWriteLockTimeoutException, which is a five-second assertTimeout that trips when the machine is loaded — it passes on its own both with and without this change.

No rush on the direction question for @sebersole, and the "improvement" reclassification makes sense to me.

[Develop-KIM]

Rebased onto main after HHH-20766 (#13174), which rewrote the recreate path in AbstractOneToManyDecomposer.

The rebase made the change smaller. The recreate rows now come from FrozenCollectionRows, which already leaves null entries out while keeping their positions, so the guard I had in decomposeRecreate is gone — the recreate test passes on plain main. The applyUpdateChanges guard went the same way: its only caller is now the interpretation == null fallback, which PreparedCollectionMutation never produces. What is left is the queued-position fix in decomposeQueuedOperations, the PersistentList change-set skipping null slots, the legacy nextIndex++, the tests and the migration-guide note.

For the record, plain main still fails 8 of the 10 tests: all 5 legacy-queue cases (compacted indices), and on the graph queue the two initialized-update cases (PropertyAccessException from the null Addition) plus the queued null ([0,1,2] instead of [0,1,3]). Full :hibernate-core:test is green on the rebased branch (17604 tests, 0 failures).

[beikov]

The changes look good to me. I'll bring this matter up today in our team meeting.

[github-actions]

Documentation Preview

This preview has been removed following the closure of this PR.

[sonarqubecloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud