line / kotlin-jdsl

Kotlin library that makes it easy to build and execute queries without generated metamodel
https://kotlin-jdsl.gitbook.io/docs/
Apache License 2.0
651 stars 85 forks source link

ListQuery With Select Distinct Requires a Non-nullable type, but returns Nulls in List #717

Closed MrThreepwood closed 1 week ago

MrThreepwood commented 1 month ago

This looks like it's just a Java interop problem, but if we do something like

listQuery<SomeClass> { 
  selectDistinct(SomeClass::class.java)

  from(OtherClass::class)
  join(OtherClass::optionalSomeClassId, JoinType.LEFT)
}

Then the results end up with a null value when an other class that does not have a some class id exists in the query. This makes sense, but the return type is wrong. Not entirely sure what the expected outcome should be, maybe the library should be filtering the response to remove nulls, maybe the return type should be nullable.

We might be able to get around this by specifying a nullable type to the listQuery generic argument, but then we can't use selectDistinct(T::class.java) because technically it's not a class of type T. Making select distinct take a class of type T? might fix it, though this would still put the onus on the person writing the code to know that their code could return a null record (which does make perfect sense in hindsight).

Overall the current behavior is a little surprising.

shouwn commented 1 month ago

Hi @MrThreepwood

I also think this is misleading to the user.

That's why Kotlin JDSL 3.0.0 changed all return types to nullable. The interface is modified so that the user needs to guarantee that they are not nullable.

Since the most of the query results in JPQL are non-null, this fix creates inconvenience for users. As a result, users have asked me several times why Kotlin JDSL 3.0.0 only returns nullable types.

However, I believe that a warning with a small inconvenience at compile time is better than an error at runtime due to incorrect nullable information, such as in the case you mentioned.

shouwn commented 1 week ago

There has been no further reaction to this issue, so I'll be closing the issue. If you have any additional thoughts on this, please let me know.