respect NO_PROXY for polymorphic associations

[gavinking]

Alright, so, I've always been against the "no proxy" style of lazy fetching, since it means you can't get the foreign key of an associated entity without fetching the entity. Which is stupid, because the FK is sitting right there.

(Even introducing a static Hibernate.getId() method would not work since the field is probably private and hidden behind a getter that would trigger fetching.)

But, on the other hand, I'm not happy at all with how we currently treat @LazyToOne(NO_PROXY). The current behavior in 6, and according to my tests (though contrary to what is claimed in a bug report), also in 5, is that for a lazy polymorphic association we just ignore NO_PROXY and fall back to using a proxy.

I don't think that can possibly be the best thing, and one of the following is what should happen. Either we should:

1. error out and make the user change the annotations,
2. respect what's been explicitly specified, or
3. just drop the whole @LazyToOne annotation and NO_PROXY altogether.

To be clear, 3 would be perfectly fine by me. But if we're going to have it, we should make sure it doesn't suck.

At first I started arguing it would be a bad idea to do lazy fetching of polymorphic associations based on field access on the owning side of the association, but actually that's a garbage argument here, since we already do it for lazy fields, and anyway, even proxies break if your entity has public fields. Plus we have "extended" bytecode enhancement where even this is perfectly safe. And we use it in Quarkus.

So, no, that was wrong: the real reason we've always disfavored this implementation is that we want efficient access to the id without hitting the database, not anything about field interception being dangerous. I remember now.

So after chatting with @beikov I think we've agreed that this can actually be implemented, but only by a funny process where:

• When an entity is instantiated, the FK is kept in the field inside a "flyweight" object. Really just the same unfetched entity instance we use now, I guess. But here we don't register it with the PC, and we're going to throw it away later.
• When the field is first accessed, we fetch the real entity, and replace the flyweight with the fetched object. This depends on a change to the bytecode enhancement, I believe.

The question is whether doing this work is worth the benefit to users, given that we don't think this is a terribly great feature to begin with. (If it were up to me, my inclination would be to just deprecate that annotation, if we're not going to sit down and do the work.)

WDYT @sebersole @beikov @Sanne ?

[gavinking]

Most of what I wrote above is garbage because it presupposed that our "no proxy" laziness was based on interception on the owner entity side. But actually that's not how it works at all, and so:

• for non-polymorphic associations, we can still get the id of a "no proxy" lazy entity, even without fetching it, and
• this sort of laziness really is inappropriate for polymorphic associations, which would need a different kind of "no proxy" laziness, and which really would have the limitations I described.

So actually, now that I understand it properly, I think the current behavior is sort-of fine.

But we should make it an error to write @ManyToOne(fetch=LAZY) @LazyToOne(NO_PROXY).

[sebersole]

But we should make it an error to write @ManyToOne(fetch=LAZY) @LazyToOne(NO_PROXY).

We actually discussed this versus "just logging" twice in recent(ish) history - once when bytecode enhancement was rewritten and then again when we added bytecode proxies.

The problem is that the original bytecode enhancement code often forced users to apply the NO_PROXY. Starting to throw an error now for something Hibernate has sometimes in the past forced users to add is not quite cut-and-dried in my opinion.

[gavinking]

Well what about if we deprecated the @LazyToOne annotation, which I don't like much to begin with, and introduced @NoProxy or something like that which would be validated in the right way?

#5588

[sebersole]

Yep, that's reasonable.

And I agree about the legacy annotation. Definitely not mine ;)

[gavinking]

Great, I'll start work on it.