Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: EXPOSED-114 Type parameter can't be inferred for EntityID with eq/neq op #1961

Merged
merged 2 commits into from
Jan 9, 2024

Conversation

bog-walk
Copy link
Member

@bog-walk bog-walk commented Jan 7, 2024

The following warning shows when a comparison operator is used between an EntityID column and a column of matching non-EntityID type:

Type argument for a type parameter S2 can't be inferred because it has incompatible upper bounds: Int, EntityID (multiple incompatible classes). This will become an error in Kotlin 2.0

This happens because there is no overload for this type of comparison, so the overload being used is this one:

infix fun <T, S1 : T?, S2 : T?> Expression<in S1>.eq(other: Expression<in S2>): Op<Boolean> { }

Appropriate overloads have been added for each relevant comparison op.

…q/neq op

An IDE warning is shown stating incompatible upper bounds (which will become an
error in later Kotlin version) when using a comparison operator between an
EntityID column and a column of matching non-EntityID type.

An overload to handle EntityID columns against other Expressions (instead of
just values) has been added for every relevant comparison operator.

@LowPriorityInOverloadResolution is added to ensure no compilation ambiguity
error (as has already been done for eq/neq).
…q/neq op

Add symmetric operators for new overloads and adjust test.
@bog-walk bog-walk force-pushed the bog-walk/fix-generics-comparison branch from 1013810 to 59354a1 Compare January 8, 2024 17:21
Comment on lines +307 to 308
@LowPriorityInOverloadResolution
infix fun <T : Comparable<T>, S : T?> ExpressionWithColumnType<in S>.less(t: T): LessOp = LessOp(this, wrap(t))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As was already done for eq() and neq() above, this is necessary to ensure the right overload is used.

@bog-walk bog-walk requested a review from e5l January 8, 2024 18:37
Copy link
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @bog-walk, great job!

LGTM

@bog-walk bog-walk merged commit 04a6c92 into main Jan 9, 2024
5 checks passed
@bog-walk bog-walk deleted the bog-walk/fix-generics-comparison branch January 9, 2024 14:26
@scottkennedy
Copy link

After updating to 0.47.0 from 0.46.0, I'm now getting:

Type argument for a type parameter E can't be inferred because it has incompatible upper bounds: UUID, EntityID (multiple incompatible classes). This will become an error in Kotlin 2.0

With this code:

val foo = uuid("foo")
val bar = uuid("bar")

fun test() {
    table.selectAll().where {
        table.foo eq table.bar
    }
}

This PR seems to be the only related change in the change log.

@bog-walk
Copy link
Member Author

bog-walk commented Feb 2, 2024

Hi @scottkennedy I came across this issue today as well and a fix will be pushed shortly.

Here is the issue ticket, EXPOSED-280, if you'd like to track it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants