hibernate-orm icon indicating copy to clipboard operation
hibernate-orm copied to clipboard

HHH-15725 Criteria API Expression.as adds cast even when the cast type is equal to the expression type

Open dreab8 opened this issue 1 year ago • 6 comments

https://hibernate.atlassian.net/browse/HHH-15725

dreab8 avatar Nov 28 '22 15:11 dreab8

@beikov what about merging this an then you can open a github discussion for the HQL support?

dreab8 avatar Jan 12 '23 09:01 dreab8

what about merging this an then you can open a github discussion for the HQL support?

I think we should at least name the expression class like the syntax construct that we would potentially use for HQL, to avoid confusion later. I think we all agree that such a function is useful for some cases in HQL (e.g. treating Timestamp as Instant), the question is, how do we want to name it. Any ideas @sebersole?

I acknowledge that naming it treat might be a bad idea, so how about treat_as? Then we could name the expression class TreatAsSqmExpression

beikov avatar Jan 12 '23 09:01 beikov

I acknowledge that naming it treat might be a bad idea, so how about treat_as? Then we could name the expression class TreatAsSqmExpression

Please, please, please no more snake case in HQL. It’s the ugliest thing ever to see a mix of camel case and snake case in the same language.

(We already have a problem there with some of the aggregate functions we added recently, but I’m accepting that because that’s the standard naming on SQL. But this isn’t a standard SQL construct so we should stick to our convention which is Java naming conventions.)

But anyway I don’t understand: how is this not just a flavor of cast( as )?

gavinking avatar Jan 12 '23 10:01 gavinking

But anyway I don’t understand: how is this not just a flavor of cast( as )?

So look, AFAICS, there's no need for this operation in HQL. Why not? Because in HQL we already have a cast() function, which JPQL does not have.

Yes, it's a bit weird that the criteria API has its own essentially-unspecified typecasting operation, and honestly I'm not clear on quite how Mike managed to sneak that one in there, but we should consider it to be a synonym for cast(), not a separate new thing.

If we have a typecast that only exists at the level of the HQL type system, and not in the database, then we can still express that using cast( ... as .. ), for example, in:

select new MyDto(e.id, cast(e.created as Instant)) from MyEntity e

we should just be smart enough to detect that no typecast is needed, and optimize it away.

gavinking avatar Jan 12 '23 10:01 gavinking

we should just be smart enough to detect that no typecast is needed, and optimize it away.

I think we can do this as well and that we should model this through the cast optimization you mentioned, but this is where @sebersole said, that it might be surprising for users when they write a cast explicitly in HQL, but it doesn't show up as cast in SQL.

beikov avatar Jan 12 '23 12:01 beikov

it might be surprising for users when they write a cast explicitly in HQL, but it doesn't show up as cast in SQL.

I mean, I dunno, it might be equally-surprising to see a timestamp getting unnecessarily cast to timestamp...

gavinking avatar Jan 12 '23 14:01 gavinking