Thanks for your pull request!

Your pull request does not follow our editorial rules. Could you have a look?

• title should preferably start with an uppercase character (if it makes sense!)

_{This message is automatically generated by a bot.}

/cc @gsmet (hibernate-orm)

Great idea - tagging @FroMage for awareness

😭 Deploy PR Preview failed.

This sounds like we should have a @WithLazyTransaction on REST endpoints, to alleviate this issue. Kinda like HR has implicit @WithLazySession.

BTW, shouldn't we also require transactions for HR, then? Just to be consistent.

BTW, shouldn't we also require transactions for HR, then? Just to be consistent.

Perhaps yes, but bear in mind in that case the mechanism is quite different as it wouldn't be JTA. We'd need to think of the consequences of that - I'd be especially more concerned on starting them automatically (w/o an explicit opt-in) as we'd not want to give a false sense of safety w/o the user understanding the implications.

This sounds like we should have a @WithLazyTransaction on REST endpoints, to alleviate this issue. Kinda like HR has implicit @WithLazySession.

Not sure why... ? Transactions are started lazily, no? Meaning on first connection request, which with Hibernate would happen on first load or flush?
@Transactional is not meant to start a transaction, just to delimit the transaction boundaries.

In any case, I strongly oppose adding many different annotations without thinking of a unified approach. The current state of things is quite confusing already.

Ah, so @Transactional is lazy already? I guess I forgot that.

So, hold on. In ORM, the session is lazy, and the TX is lazy. They're only opened when used by API.

Except, before this patch, we used to be able to access ORM for Read-Only operations.

Now, this means we can never use ORM from outside of an endpoint annotated with @Transactional.

This is becoming a 2-factor API: you can only use it with an annotation and using its API.

I do think we are going to have to talk about simplifying access to the ORM API, even though this can be done later, but perhaps REST endpoints by default should have an automatic @Transactional, assuming this works for both ORM and HR. In the future.

This is becoming a 2-factor API: you can only use it with an annotation and using its API.

Technically you can also start a transaction programmatically with QuarkusTransaction. But yes, essentially, it is 2-factor: you need to specify the scope of whatever session you're using, and that requires specifying the transaction boundaries.

I do think we are going to have to talk about simplifying access to the ORM API, even though this can be done later, but perhaps REST endpoints by default should have an automatic @Transactional, assuming this works for both ORM and HR. In the future.

That would cause a lot of backwards incompatibility since it would effectively extend the scope of @Transactional methods called by REST endpoints.

We could instead have some sort of "last-minute" default transaction scoping that, assuming a request scope exists and no transaction scope exists when first accessing the Session, would start a transaction scope automatically and make sure it ends (commit/rollback) when the request scope ends.

But:

1. It would work for Hibernate, but not for direct use of JDBC or Reactive SQL clients -- since we allow no-transaction access there.
2. It feels very, very magic at this point. So much so that I shudder to think of the experience of a user trying to debug problems related to this behavior.
3. Alternatively if you want to keep things simple you can just put @Transactional on your REST resources, at the class level. And then you're all set. It's not that complex.

I do think we are going to have to talk about simplifying access to the ORM API, even though this can be done later, but perhaps REST endpoints by default should have an automatic @Transactional, assuming this works for both ORM and HR. In the future.

That would cause a lot of backwards incompatibility since it would effectively extend the scope of @Transactional methods called by REST endpoints.

Yeah, because @Transactional defaults to REQUIRED instead of REQUIRES_NEW. True.

Well, the current PR is also backwards incompatible 🤷

1. It would work for Hibernate, but not for direct use of JDBC or Reactive SQL clients -- since we allow no-transaction access there.

They use @Transactional? they don't use Hibernate Session though. So I'm not sure they're in scope for this change.

2. It feels very, very magic at this point. So much so that I shudder to think of the experience of a user trying to debug problems related to this behavior.

This is essentially the default for HR and REST, no? If you don't specify a transaction/session, one will be provided for you at the request scope. Seems fairly straightforward and helpful to me.

3. Alternatively if you want to keep things simple you can just put @Transactional on your REST resources, at the class level. And then you're all set. It's not that complex.

Except you need to do it on every REST resource, and it requires the user to first get an exception before acting on it. This is boilerplate. You're thinking about this with your expert hat, not your newbie hat. Which is good, it's useful, but you need both hats on :)

Anyway, I think this is mostly off-topic for this particular PR, sorry to have diverged on the topic.

@yrodiere What's the status on this one?

@yrodiere What's the status on this one?

Many tests failing, as you can see.

@Sanne suggested we'd better be damn sure use all use cases work fine with transactions before we do this, which I agree with. There are still edge cases where not using a transaction is... arguably correct.

So, we'll need to address a good chunk of #51243 before we flip the switch here.