[hibernate-dev] Extended KEY expression support

[Steve Ebersole]

My initial thoughts...

On Fri, Jan 27, 2017 at 1:29 PM Christian Beikov <christian.beikov at gmail.com>
wrote:

> I have a little proposal for supporting the use of a KEY expression in
> the FROM clause and I'd like to hear your opinions on that.
> Unfortunately the JPA spec does not support that, but since a key of a
> java.util.Map mapping may be an entity type, we need to specify how one
> can "join" that key explicitly.
>

> Right now(pre HHH-10537), when joining a java.util.Map mapping, an inner
> join is generated for the map key entity type. In my fix for HHH-10537 I
> changed the behavior to respect/inherit the join type of the collection
> join.
> The problem is, that one can't further join any attributes on that key,
> because there is no syntax in the HQL or the JPA spec that allows to do
> that.
>
> We need to decide (1) whether we always want to join the entity key or
> require the user to do that specifically via e.g. something like "JOIN
> alias.map m JOIN KEY(m) k"
> and also (2) how the syntax for joining further attributes should look
> like. If we decide to not allow the "JOIN KEY(m)" syntax for (1) we have
> to support something like "JOIN KEY(m).association", otherwise we can
> just use the alias like for normal joins "JOIN k.association".
>

I definitely like the syntax to join the key if it happens to be an entity
(or an embedded!).  That makes the syntax for further referencing or
joining it's attributes very natural.

Either way, we have to change the grammar but I'd rather like to
> support/implement the map key joining syntax like "JOIN KEY(m) k" for
> (1).


By "change the grammar" I assume you mean the JPA query BNF?  Because the
SQM grammar already supports this.  Or perhaps you mean the pre-6.0
Hibernate grammar?  If so, see below.

A further change to that would be to not generate the implicit key
> table join anymore but require the user to do the join explicitly. Since
> that would break backwards compatibility, I'd like to make that behavior
> configurable and of course, by default it will generate the implicit key
> join to maintain backwards compatibility. I also propose to switch the
> default in 6.0 so that the join is not generate anymore.
>

I personally would vote to *not* make this change in 5.x even in terms of
making it configurable.  The grammar there is fugly enough already and this
is the kind of thing (especially making branches of it configurable) that
takes that fugliness to a new level.

But I think it is a useful change and would certainly consider it for 6.0



> The usage in the JPA Criteria API will unfortunately require a cast
> since the return type of javax.persistence.metamodel.MapAttribute#key()
> is javax.persistence.metamodel.Path instead of
> javax.persistence.metamodel.SingularAttribute but essentially the same
> functionality is available to a user out of the box.
>

Right.  In terms of SQM and 6.0 this perfectly illustrates the importance
of the Navigable/NavigableSource contracts and defining references and
joins based on them as opposed to strictly attribute-based.

I *hate* the idea of making the map key "pose" as a SingularAttribute just
for the sake of doing things we should be able to do with it.  We have to
cast anyway, so I'd much prefer to cast to Hibernate-specific (this is
Hibernate specific anyway) contracts that expose this more properly
probably based on Navigable/NavigableSource above.

In those terms, a map key is always a Navigable.  If the key happens to be
an entity (or an embedded!) then the key is also a NavigableSource and can
therefore be further dereferenced.  I'd rather base such an extension on
this concept rather than making map key be something it is not just to make
it work.  We can make it work and model it correctly!


Specifying a custom join for a key would look like this in the Criteria API
>
> MapAttribute<Entity, MapKeyEntity, MapValueEntity> mapAttribute = ...
> Join<Entity, MapKeyEntity> keyEntity =
> mapAttribute.join((SingularAttribute<? super Entity, ? extends
> MapKeyEntity>) mapAttribute.key(), JoinType.LEFT);
> keyEntity.join(...)
>
> So the questions again.
>   1. Do you all agree that this is important and should be done?

  2. Agree to not generate implicit joins for keys in future versions?

  3. Allow joining the key in a separate join?
>

I am missing how 1, 2 and 3 are any different.  Anyway...

I think it is useful.  I think we should plan to do it... for 6.0


>   4. Allow further joins on a key?
>

And this is a natural consequence of 1, 2 and 3 ;)


>   5. Happy with how it can be done in JPA Criteria?
>

Meh.  JPA is a spec.  We can make proposals, but we are not the only ones
on the EG.  What is more important to me is that we make it possible from
*our* implementation of the spec.

Especially moving forward considering we dropped the legacy criteria API
and plan to develop criteria extensions on top of the JPA criteria API.  It
is overall better designed - we just need to add in our extras that JPA
does not support.  What that looks like exactly (to get around JPA not
defining it) is not very as relevant to me.  We'll make the proposal which
is the most we can do.



> In addition to that, it would be nice if anyone could make someone from
> the JPA EG aware of this.
>
 From a JPQL BNF point of view, I'd propose the following changes
>
> from
>
> join_single_valued_path_expression::=
>
> identification_variable.{single_valued_embeddable_object_field.}*single_valued_object_field
>
> to
>
> join_single_valued_path_expression::=
>
> identification_variable.{single_valued_embeddable_object_field.}*single_valued_object_field
> |
>      map_field_identification_variable
>
>
Well considering myself, Scott and Emmanuel are all part of the JPA EG...
done ;)

That said, in my experience, the JPA EG (or certain members of it) tend to
be extremely resistent to changes.  I'm not certain this is something they
would go for, specificaly because of the Criteria implication you mention.
While I'm not sure it says this specifically, the assumption is that the
BNF applies to both JPQL and Criteria.  We can certainly bring it up for
discussion if/when we have serious discussions about the "next steps" for
the spec.