Generic metamodel attribute should be resolved as concrete as possible

[quaff]

Given

	@Entity
	public class Book extends OwnerContainer<Owner> {

		@Id
		private Long id;
	}

	@Entity
	public class Owner {
		@Id
		private Long id;
	}

	@MappedSuperclass
	public class OwnerContainer<T> {

		@ManyToOne
		T owner;
	}

Then

entityManager.getMetamodel().entity(Book.class).getAttribute("owner").getJavaType()

Should return Owner.class instead of erased type Object.class

see hibernate/hibernate-orm#7630 (comment)

[gavinking]

Well, that would be a breaking change, so we can't do that. Also, to handle generics correctly, the method would need to return a Type, not Class.

So it would have to be a new method.

But honestly I just don't see this as a very high priority at all. Entities don't usually benefit from parametric polymorphism, since they must be mapped to a database table which is not generic. In the particular example shown, it's quite easy to move the mapping for owner down to the concrete entities which implement the mapped superclass. You're not really gaining much here.

[quaff]

that would be a breaking change

Let's discuss the real impact of this change, I think it's a enhancement instead of breaking change.

the method would need to return a Type, not Class. So it would have to be a new method.

I agree that, but it should be discussed in another feature request issue.

it's quite easy to move the mapping for owner down to the concrete entities which implement the mapped superclass.

Moving the field down add lots of boilerplate code is not elegant, and lots of existing code need changes to adapt the breaking change which introduced by Hibernate 6.

[gavinking]

I think you're underestimating this. If it's worth doing, it's worth doing properly.

First of all, if we're going to have metamodel objects representing actualized versions of inherited members, the first thing I want to know is: what is the way to get hold of these things via the static metamodel. Now, that's a solvable problem, but we would have to solve it.

Second, consider the signature of this method:

Attribute<? super X, ?> getAttribute(String name);

That's actually wrong for the operation you're proposing. The correct signature would be:

Attribute<X, ?> getActualAttribute(String name);

without the wildcard.

Now, perhaps it's not that big of a deal, but it's still a whole new operation, along with the addition of an operation on Attribute returning Type.

So in the end that's—just right off the top of my head—much more than a trivial change for something that's of pretty marginal value.

[quaff]

"as concrete as possible" as title said, If it's not possible then fall back to current behavior.

[beikov]

So you would rather have

entityManager.getMetamodel().entity(Book.class).getAttribute("owner").getJavaType()

return the concrete type i.e. Owner.class if OwnerContainer is only ever extended with specifying Owner as type variable, but start returning Object.class as soon as there is another subclass passing a different type as type variable?
Sorry, but I think this is horrible. Hibernate ORM 5 actually suffers a bug where it randomly uses one of the possible type variable assignments. I guess you might have relied on this broken behavior, but as soon as you have more type variable assignments you will see the problem.

If the JPA spec ought to handle generics, it will need a new API for that.

[quaff]

but start returning Object.class as soon as there is another subclass passing a different type as type variable?

I'm not sure I understand what you means, for other subclass such as House:

	@Entity
	public class House extends OwnerContainer<HouseOwner> {

		@Id
		private Long id;
	}

	@Entity
	public class HouseOwner {
		@Id
		private Long id;
	}

then

entityManager.getMetamodel().entity(House.class).getAttribute("owner").getJavaType()

should return HouseOwner

[beikov]

I guess you mean

entityManager.getMetamodel().entity(House.class).getAttribute("owner").getJavaType()

should return HouseOwner?

This is where you seem to lack an understanding of how the metamodel is structured. An attribute is declared by a managed type and clearly OwnerContainer declares the owner attribute. This means that every call to getAttribute("owner"), regardless on which subtype you call it, will have to return that one attribute object. Clearly you understand that an object can only return one result, so if you want to know the attribute type based on a container subtype, there is a need for a new method.

[quaff]

I guess you mean should return HouseOwner?

Yes, It's a typo.

An attribute is declared by a managed type and clearly OwnerContainer declares the owner attribute. This means that every call to getAttribute("owner"), regardless on which subtype you call it, will have to return that one attribute object.

that's why I'm opening this issue, there is something need to improve at specification level, and It's a small fix for hibernate.

For an given entity, the owner type is concrete, and I'm querying attribute of that given entity, why not return the concrete type? If I'm querying owner attribute on OwnerContainer, the type is not concrete, but OwnerContainer is not an entity, so that will not going happen.

[quaff]

For static metamodel, It should generated like this:

@StaticMetamodel(OwnerContainer.class)
public abstract class OwnerContainer_ {

	public static volatile SingularAttribute<OwnerContainer, Object> owner;

}

@StaticMetamodel(Book.class)
public abstract class Book_ {

	public static volatile SingularAttribute<Book, Long> id;

	public static volatile SingularAttribute<Book, Owner> owner; // The missing part

}

[quaff]

I've tested with EclipseLink 3.0.4, It does return the expected Owner.class instead of erased type Object.class, so I guess there is no technical limitation you worried.
I think the spec should be revised to clarify the expected behavior, WDYT @gavinking

[gavinking]

Not a single person here claimed that it cannot be done.

The claim is that it would be wrong and inelegant to do so.

As I've already said above, it's pretty clear from the signature of the method (i.e. from its return type) that that operation is supposed to return the member as declared by the supertype. See that wildcard there? It's there for a reason, not just for decoration!

If the operation were supposed to behave the way you claim it should behave, that is, if it were supposed to return the member as actually inherited by the subtype then its signature would be Attribute<X, ?> getAttribute(String name) and 6.2.1.1 of the spec would read:

For every persistent non-collection-valued attribute y declared [or inherited] by class X, where the type of y is Y, the metamodel class must contain a declaration as follows:

   public static volatile SingularAttribute<X, Y> y;

But the words "or inherited" just aren't there. I added them in right now.

Now, do the spec and Javadoc leave some ambiguity here, well, yeah, sure they do, and that's a bit unfortunate. But I insist that the most reasonable interpretation is that the JPA metamodel works just like the Java reflection API in this aspect.

So sure, this issue can be solved: properly, by introducing a new operation with the correct signature, in a future version of JPA (i.e. Attribute<X, ?> getActualAttribute(String name); as above).

[quaff]

IMO, there is no need to introduce a new method getActualAttribute(), It means I want the actual attribute if I called getAttribute(), I cannot imagine which use case would want the non-actual attribute.

I repeat my request in a nutshell:

1. Revert the behavior of Hibernate 6 to Hibernate 5, and it's aligned with EclipseLink, developers can benefit from the consistency.
2. Revise the spec to clarify that behavior, and state that generated static metamodel should add concrete attribute as I said Generic metamodel attribute should be resolved as concrete as possible #562 (comment)

Wish other developers state their opinions here too.