jdaugherty commented on PR #16344: URL: https://github.com/apache/grails-core/pull/16344#issuecomment-5715541879
@matrei thanks for the thorough round. Finding 1 is fixed in d2f76c9e3b. **Fix.** `HibernateGormInstanceApi.lockRow` on Hibernate 7 now resolves the hierarchy root entity from the mapping metamodel and runs the scalar lock query against it, so the row that holds the version (and that `lock()` contends on) is locked before the reload. The instance form, the polymorphic root load and the static `lock(id, refresh: true)` form all go through it. **Tests.** Both specs gain a `tablePerHierarchy false` root/subclass pair and a competing-commit feature in the shape you described: a raw JDBC competitor updates the root row (`title`, `version = version + 1`) and holds its transaction open; the locked refresh must block, then reload the committed state and version once the competitor commits, then save. Four rows: loaded as the subclass, loaded polymorphically through the root, and static `lock(id, refresh: true)` on each class. Before the fix three of the four Hibernate 7 rows failed with `No rows were returned from JDBC query for versioned entity`; the one that passed was the static call on the root class, whose instance API already targeted the root table. All four pass on both versions now. The JDBC connection and table-name lookup are shared helpers in each spec, and the existing `FOR UPDATE` contender check uses them. **Runs** (`DO_NOT_CACHE_TESTS=1`, `--no-build-cache`): | Run | Result | |---|---| | `grails-data-hibernate7-core:test` (full, incl. `Hibernate7RefreshLockSpec` 72/72) | 3151 tests, 0 failures, 29 skipped | | `grails-data-hibernate5-core:test` (full, incl. `Hibernate5RefreshLockSpec` 73/73) | 911 tests, 0 failures, 39 skipped | | `codeStyle` on both modules | clean | **Finding 2.** Left as-is for now. Writing the entry lock mode directly would save the round trip but needs proxy unwrapping and a second internal-SPI write path, and as you note the cost is small next to the lock wait. I did check whether the final `session.lock` cascades to associations on the cascade parent and it does not (the child stays at `READ`), so there is no behavioral reason against it if we want to pick it up later. Ready for another look. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
