Consider adding typed version of query on FullTextEntityManager

Description

Today code change from JPA are a bit unfavorable.

Environment

None

Activity

Show:
Emmanuel Bernard
January 17, 2017, 1:05 PM

ah, forgot.
I'd use Object[] in the type then

Yoann Rodière
January 17, 2017, 2:07 PM

To be more precise...

The problem I was referring to is that, currently, setProjection mutates the FullTextQuery, making the type returned by a given FullTextQuery change at runtime.

For instance:

I agree there is a user mistake in this code, but the purpose of typing is precisely to avoid that kind of mistake, so we're not really making our API better here.

As discussed on HipChat, we could avoid this issue by removing setProjection and replacing it with a non-mutating method that would return a new instance, let's say ".withProjection":

Which would then be, with correct code:

Assuming setResultTransformer (which has the exact same problem) was fixed the same way (by making ResultTransformer generic), we'd still have an issue:

Also, we'd have yet another issue with the behavior described in (see the comments there).

So... Yes, I think we'll need to think about it

Sanne Grinovero
January 25, 2017, 4:58 PM

Notes from discussion with Steve Ebersole:

Conceptually in the JPA / ORM API, the "createQuery" method parameter shouldn't express the "from" but the output type of the query. For example with an HQL query, ORM will check at runtime that the actual expected return type of the query target is compatible with the type which the user requested.

I agree with this view, which implies that we need to change our method. We should be able to return a Query<T> when the projection is specified as being T; with other kinds of projects, we might be able to suggest a return type Query<Object[]>

In the short term we will just return "Query<Object>" - as it always essentially did since it wasn't parameterised - to be backwards compatible

Yoann Rodière
January 25, 2017, 5:13 PM

We had a discussion regarding setResultTransformer with Steve Ebersole, and basically what's wrong in my previous comment is that the type parameter T in Query<T> is set from the start and cannot change, even if you call setResultTransformer. Basically there is a constraint on setResultTransformer that only allows to use a ResultTransformer<T>, and T is the output type for the ResultTransformer, which will always get passed an Object[] as input.

Also, it won't be setResultTransformer but setTupleTransformer or something like that, but it's another matter.

Anyway... in our case, here are the solutions we could implement:

  • to only ever use FullTextQuery<Object> as our return type

  • or (it makes more sense) to make FullTextQuery not generic, extending Query<Object>

  • or to use the "with" kind of method mentioned above, which would not change the query's return type but return a new query with a different return type (downside: users might expect additional changes to the original query to affect the new one).

  • or to make users give us information about whether they want to project or not from the beginning. We would have two different ways to retrieve the query: FullTextQuery<Object[]> query = session.createFullTextProjectionQuery(Foo.class, Bar.class).projection("fooField", "barField") or FullTextQuery<FooBarCommonParentType> query = session.createFullTextQuery(Foo.class, Bar.class).

Yoann Rodière
April 25, 2018, 7:36 AM
Edited

Resolved as part of the Search 6 proof of concept groundwork.
Followed up by HSEARCH-3093.

Assignee

Yoann Rodière

Reporter

Emmanuel Bernard

Labels

None

Suitable for new contributors

None

Pull Request

None

Feedback Requested

None

Fix versions

Priority

Blocker
Configure