gustavoharff opened a new pull request, #16559:
URL: https://github.com/apache/grails-core/pull/16559
## Description
In the Hibernate 7 criteria builder, an association block nested in another
association block ignores the join type it asks for and is always an inner
join. With `1` standing for a left join, as in the rest of the criteria DSL:
```groovy
Pet.createCriteria().list {
owner(1) {
face(1) {
or {
isNull('id')
eq('name', 'Homer')
}
}
}
}
```
renders `left join person ... join face ...`, so every pet whose owner has
no face, and every pet without an owner, is dropped. Hibernate 5 renders a left
outer join for both levels and returns them.
The join type of a nested block is recorded on the criteria of the block
that contains it, but `PredicateGenerator` joined the nested association with
`From.join(name)`, whose default is an inner join, without reading it. Only the
association blocks of the queried class got their join type, through
`JpaCriteriaQueryCreator.registerDetachedJoins`.
A related problem showed up in the same code: an association block reused
any join of the same name made by an outer context. In
```groovy
Pet.createCriteria().list {
face { eq('name', 'Santa') }
owner(1) { face(1) { ... } }
}
```
the `face` block inside `owner` restricted the pet's face join instead of
joining the owner's face, depending on the order of the blocks, and an
association block of a subquery could reuse the join of the outer query.
Changes:
- `handleAssociationCriteria` records the join types of the nested blocks on
the nested context (`JpaQueryContext.registerJoinType`) and joins each nested
association with its recorded join type, an inner join when none is recorded.
- The join types are recorded as join types, not as aliases, so a criterion
on a property of the same name, such as `isNotEmpty('pets')` next to `pets {
... }`, still resolves to the property.
- An association block reuses only the joins of its own context
(`getLocalFrom`, `getLocalAliasDefinition`), so a join that an outer block or
an outer query made for an association of the same name is never taken for it.
No issue was opened for this: it showed up while moving an application's
criteria queries from Hibernate 5 to Hibernate 7. There is no documentation
change, since the guide does not describe the join types of association blocks
and this restores the Hibernate 5 behaviour.
## Testing
- New `HibernateCriteriaBuilderNestedJoinTypeSpec` (end to end through the
criteria DSL, asserting both the rows and the generated joins): nested left
joins, a criterion inside a nested left joined block, a nested block without a
join type staying an inner join, a projection over a nested left join, blocks
of the same name at two levels in either order, a detached criteria, a
subquery, and a collection criterion next to its block.
- Six of its eight cases fail on `8.0.x` without this change; the other two
(a nested block without a join type, a collection criterion next to its block)
guard behaviour that must not change.
- Extended `AliasRegistrySpec`, `JoinTrackerSpec` and `JpaQueryContextSpec`
for the local lookups and the recorded join types.
- `./gradlew clean aggregateViolations --continue`: no Checkstyle, CodeNarc,
PMD or repository convention violations.
- All passing:
- `:grails-data-hibernate7-core:test`: 3662 tests, 0 failures
- `:grails-data-hibernate7:test`
- `:grails-data-hibernate7-spring-boot:test`
- `:grails-data-hibernate7-dbmigration:test`
- `:grails-data-hibernate7-dbmigration-core:test`
- `:grails-test-examples-hibernate7-criteria-extension:integrationTest`
- `:grails-test-examples-hibernate7-gorm:integrationTest`
- The full `./gradlew build --rerun-tasks` was not run; the change is
confined to `grails-data-hibernate7`.
## Contributor Checklist
### Issue and Scope
- [ ] This PR is linked to an existing issue that has been **acknowledged or
approved** by the project team. If no approved issue exists, please give
background on why this change is necessary. Tickets are preferred for release
change log history.
- [ ] This PR addresses the **complete scope** of the linked issue. Partial
implementations or unfinished work should not be submitted for review.
- [x] This PR contains a **single, focused change**. Unrelated changes
should be submitted as separate pull requests.
- [x] This PR targets the **correct branch** for the type of change:
- **Patch release branches** (e.g., `7.0.x`): Bug fixes only. No new
features or API changes.
- **Minor release branches** (e.g., `7.1.x`): New features are welcome,
but breaking existing APIs must be avoided.
- **Major release branches** (e.g., `8.0.x`): Reserved for major
changes. Breaking API changes are permitted.
### Code Quality
- [x] I have **added or updated tests** that cover the changes introduced in
this PR. All code contributions are expected to include appropriate test
coverage.
- [ ] I have verified that all existing tests pass by running `./gradlew
build --rerun-tasks`.
- [x] My code follows the project's **code style** guidelines. I have run
`./gradlew codeStyle` and resolved any violations. See [Code
Style](../CONTRIBUTING.md#code-style) for details.
- [x] This PR does **not** include mass reformatting, style-only changes, or
large-scale refactoring unless it was **explicitly approved** in the linked
issue. Unsolicited reformatting will not be accepted.
- [x] If generative AI tooling was used in preparing this contribution, a
quality model was used to ensure contributions are **consistent with the
project's quality standards**.
### Licensing and Attribution
- [x] All contributed code is provided under the [Apache License
2.0](https://www.apache.org/licenses/LICENSE-2.0), and new source files include
the appropriate **Apache license header**.
- [x] I have the necessary rights to submit this contribution and confirm it
is my own original work (see [Legal
Notice](../CONTRIBUTING.md#i-want-to-contribute)).
- [x] If generative AI tooling was used in preparing this contribution, I
have followed the [Apache Software Foundation's policy on generative
tooling](https://www.apache.org/legal/generative-tooling.html) and have
properly attributed its use.
### Documentation
- [ ] If this PR introduces user-facing changes, I have included or updated
the relevant documentation.
- [ ] If this PR adds a new feature, I have updated the **What's New**
section of the Grails Guide.
- [ ] If this PR introduces breaking changes or changes that require user
action during an upgrade, I have updated the **Upgrade Notes** for the
corresponding version in the Grails Guide.
- [x] The PR description clearly explains **what** was changed and **why**.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
