Re: [hibernate-dev] CriteriaBuilder, aggregation and types

Hi, We discussed a bit more with Fabio on Friday and we ended up discovering that we have an issue with most Expressions that contain nested Expressions. The Renderable contract is defined with these 3 methods: String render(RenderingContext renderingContext); default String renderProjection(RenderingContext renderingContext) { return render( renderingContext ); } default String renderGroupBy(RenderingContext renderingContext) { return render( renderingContext ); } The issue is that most of the Expressions containting nested Expressions only implement render(). This means that you basically do the following: call renderProjection() -> expression1 only implement render() -> we call render() on the nested expressions instead of renderProjection(). One possible workaround is to do what was done in SearchedCaseExpression for all the Expressions containing nested expressions (e.g. implement the 3 methods and have a private method taking a lambda to propagate the variation). You can see an example of this change here: https://github.com/hibernate/hibernate-orm/pull/2568/files#diff-8fcc837a7... . Note that it has to be done for *all* the Expressions containing nested expressions. Either that or we simplify the Renderable contract to have only one render() method and a parameter defining the context. That would allow to avoid all these changes. Thoughts? -- Guillaume On Thu, Oct 11, 2018 at 4:01 PM Guillaume Smet <guillaume.smet(a)gmail.com&gt; wrote: > Hi, > > We have an interesting test case here > https://hibernate.atlassian.net/browse/HHH-13001 about the use of > LiteralHandlingMode with the criteria builder. It turns out that the > problem is a bit more general than that. > > Let's take this (not so useful) example: > query.multiselect( > document.get( "id" ), > cb.sum( cb.parameter( Long.class ) ) ) > .groupBy( document.get( "id" ) ); > > It will lead to a NPE when trying to determine the return type of the sum > in: > > https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src... > > The fact is that we totally lose the type information along the way. > > I don't know if it's something addressed in 6 and if we should try to fix > it in 5.4. In any case, I think having a workaround would be nice. > > There are a few paths we could follow: > 1/ at least throw a more meaningful error and provide a workaround. I > thought about forcing the use of the cast function but it doesn't work as > we have an optimization not adding the cast function is the type is > corresponding (see > https://github.com/hibernate/hibernate-orm/blob/master/hibernate-core/src... ). > If we remove this one and provide a helpful error message, that could help. > Note that I don't know if it's just an optimization or something mandated > by the spec itself. > > 2/ we could try to add a cast automatically for these functions needing a > proper type only when strictly necessary e.g. when you have a parameter (or > a LiteralExpression transformed to a parameter). That would mean adding a > method ExpressionImplementor (and change all our code to use > ExpressionImplementor as it currently uses the JPA version mostly > everywhere). That would be more transparent for the user but clearly a > significant amount of (dumb) work/changes. And probably a nightmare to > merge in 6 if this area has changed. > > Thoughts? > > -- > Guillaume > _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev