HHH-17743 Allow updates outside transaction

[subijayb]

Please check https://discourse.hibernate.org/t/jakarta-persistence-transactionrequiredexception-on-query-modifying-and-transactional-methods/9042

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

[beikov]

Please also add a test that reproduces the problem to the test class org.hibernate.orm.test.flush.NonTransactionalDataAccessTest.

[hibernate-github-bot]

Thanks for your pull request!

This pull request appears to follow the contribution rules.

› This message was automatically generated.

[subijayb]

Hi @beikov , could you please review the test case ?
I don't know why I cannot assign a reviewer

[beikov]

Please rebase and squash to a single commit

[subijayb]

Hi @beikov, could you please check now and let me know the next step

[subijayb]

Hi @beikov , could you please review and let me know the next steps?

[miles-po]

Hi @beikov. This is a showstopper bug when working with the latest version of Spring Boot and JPA.

[sebersole]

Doing things outside a transation is never a show stopper ;)

Also, why is this targetting 6.4 and not main? Does this not happen on main?

[subijayb]

Hi @sebersole ,

This bug impacts all 6.x branch. I pushed it to 6.4 hoping that 6.4.5 will come out soon and we can use this. I will also merge it to main branch.
Could you please review and merge this change?

[miles-po]

@sebersole Yes, my comment was intentionally provocative due to lack of response on what looks like a viable patch for three weeks. For reference, this shows up even when trying to execute queries from the EntityManager, even native queries. With or without @Transactional. The default deleteById works, but any other deleteBy variant hits this code path. em.getTransaction() doesn't work either because it's a managed em. It appears like our only option is to completely bypass the ORM and make queries on a raw DataSource. That option is unappealing considering a very simple patch already exists here upstream.

[beikov]

Sorry this takes so long, but please understand that the ORM team has it's own priorities and is not that huge to respond to every request so fast. We're all trying our best, but if you need guaranteed fast response times, consider becoming a Red Hat customer and create support tickets, because such tasks have priority over everything else that we do.

[miles-po]

Understood. Thank you for your efforts.

[subijayb]

Thanks @beikov for merging. Could you please let know how to push it to main branch? I have to raise a new PR?

[sebersole]

2 PRs is fine. Or (future) send a PR against main and we will port it to maintained branches generally

[dreab8]

@beikov
instead of removing
getSession().prepareForQueryExecution(true)

should not be something like
getSession().prepareForQueryExecution(false)

or

getSession().prepareForQueryExecution(
				requiresTxn( getQueryOptions().getLockOptions().findGreatestLockMode() ) )

[beikov]

I already merged it on main.

@dreab8 the change of this PR is fine IMO, because doExecuteUpdate() is only ever called from executeUpdate() which already does the following:

getSession().checkTransactionNeededForUpdateOperation( "Executing an update/delete query" );