Skip to content

HHH-15142 Reuse CriteriaQuery with explicit and implicit (=literal) params #4918

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 2 commits into from
Closed

HHH-15142 Reuse CriteriaQuery with explicit and implicit (=literal) params #4918

wants to merge 2 commits into from

Conversation

ettavolt
Copy link
Contributor

@ettavolt ettavolt commented Mar 28, 2022 •
edited by beikov
Loading

Caused by pull request #4864.
Reverting a couple of noop changes too.

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

@hibernate-github-bot
Copy link

hibernate-github-bot bot commented Mar 28, 2022 •
edited
Loading

Thanks for your pull request!

This pull request does not follow the contribution rules. Could you have a look?

❌ All commit messages should start with a JIRA issue key matching pattern HHH-\d+
    ↳ Offending commits: [2ab6ccf]

› This message was automatically generated.

@ettavolt
Copy link
Contributor Author

ettavolt commented Mar 28, 2022 •
edited
Loading

I'm not sure, which issue key to put on 'no @Jpa' commit. Any advice?

@ettavolt ettavolt mentioned this pull request Mar 28, 2022
Closed
@beikov
Copy link
Member

beikov commented Mar 29, 2022

No issue key. The tests are working fine with JUnit Jupiter as you can see in the GitHub actions runs, so please revert the test changes you did and use the JUnit 5 API for new tests.

@ettavolt
Copy link
Contributor Author

ettavolt commented Mar 29, 2022 •
edited
Loading

I've downloaded the logs from your link, run grep -r CriteriaUpdateWithParametersTest, and observed no match. (No case-insensitive match to 'jupiter' either)

More over, ci/build-github.sh delegates to ci/build.sh, which in turn just calls gradle check.
When I was calling gradle check locally before my changes, I've observed no report on CriteriaUpdateWithParametersTest either. Even more: I've reverted the #4864's change in ParameterExpression, run gradle check again and got it passed. Only after I've migrated CriteriaUpdateWithParametersTest it started to fail.

@dreab8
Copy link
Member

dreab8 commented Mar 29, 2022 •
edited
Loading

I'm working on a different solution that fixes also ReuseCriteriaWithMixedParametersTest#predicateReuse, and I'll give a look also at @JPA, it seems that these tests are not executed when run with gradle.

bbortt added a commit to postfinance/citrus-simulator that referenced this pull request Apr 5, 2022
@bbortt
fix: prevent hibernate param mapping failure
aea95bc 
hibernate patched a "fix" which introduced a failure
in their param count. params should always be explicitly
annotated: hibernate/hibernate-orm#4918.
bbortt added a commit to postfinance/citrus-simulator that referenced this pull request Apr 5, 2022
@bbortt
fix: prevent hibernate param mapping failure
b3c1047 
hibernate patched a "fix" which introduced a failure
in their param count. params should always be explicitly
annotated: hibernate/hibernate-orm#4918.
bbortt added a commit to postfinance/citrus-simulator that referenced this pull request Apr 19, 2022
@bbortt
fix: prevent hibernate param mapping failure
238d1c3 
hibernate patched a "fix" which introduced a failure
in their param count. params should always be explicitly
annotated: hibernate/hibernate-orm#4918.
@dreab8
Copy link
Member

dreab8 commented Apr 26, 2022

Hi @ettavolt

thanks a lot for your contribution, I'm going to close this PR in favour of #4970

@dreab8 dreab8 closed this Apr 26, 2022
@ettavolt ettavolt deleted the HHH-15142-reuse-cq-mixed-params branch April 26, 2022 10:01
@ettavolt
Copy link
Contributor Author

Hello Andrea
I'm happy to provide something useful for the Hibernate project.
May I humbly ask for an advice to bring #3057 on the maintainers' radar? 😄

@dreab8
Copy link
Member

dreab8 commented Apr 26, 2022

Hi @ettavolt,

I'm going to give a look at it in the next few days!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ettavolt @beikov @dreab8