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

Improve SqlFunctionExpr type resolver to specify the type of the parameters #4133

Open
griffio opened this issue May 3, 2023 · 2 comments
Open
Labels

Comments

@griffio
Copy link
Contributor

griffio commented May 3, 2023

Description

Given the definition of an existing function in a Dialect TypeResolver (SqlFunctionExpr).
Only the return type can be specified.
e.g

"length" -> {
      IntermediateType(INTEGER).nullableIf(exprList.any { it.type().javaType.isNullable })
    }

The below query should be supported but fails to compile

selectLength:
  SELECT length(?);
val length = queries.selectLength("abc").executeAsOne()

results in mismatch: inferred type is String but Long? was expected

@hfhbd provides some background

we currently don't really have a good representation of a function, including the typed parameters. At the moment, we only >>specify the return type. The added Kotlin parameter type inference needs other typed sql parameters. In the past, to >>specify the kotlin type, you need to use a CAST. Ideally, we need a better function representation to specify the type of the >>parameters.

Related issues:
#3416
#3407

@griffio griffio added the feature label May 3, 2023
@hfhbd
Copy link
Collaborator

hfhbd commented May 3, 2023

Thank you for creating the issue.

This issue needs some design, and the biggest blocker is sql itself. Sql isn't strongly typed, but Kotlin is. For example, date_trunc. This function accepts a timestamp without a time zone and a timestamp with a time zone. Most dialects use two different Kotlin types, eg LocalDateTime and Instant.

So which type should the compiler use for the bind parameter? Timestamp or timestamp with time zone? Or should the compiler support both, creating a powerset of all combinations?

(Or should we wait until Kotlin supports union types someday, if ever?)

@shellderp
Copy link
Contributor

Regarding strong typing, isn't this just function overloads?

fun date_trunc(timestamp)
fun date_trunc(timestamp_tz)

However, creating overloads for Kotlin generated methods would lead to explosion in overloads for queries with many parameters.

Another option is to create a sealed class that accepts either type for this parameter. Ideally this would be per function defined in the dialect, not per query using this function, or you'd get a lot of extra classes.

The last option is just to use Any, perfect is the enemy of good, better to have some way to use these functions than nothing.

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

No branches or pull requests

3 participants