Hi everyone, I’ve just submitted a new pull reque...

# komapper

[Toshihiro Nakamura]

Hi everyone,

I’ve just submitted a new pull request that introduces a feature aimed at improving the way Komapper handles SQL templates. The new feature performs compile-time checks for SQL template syntax and parameter bindings, addressing some limitations we’ve encountered in the current approach.

In this PR, I’ve introduced the concept of commands, which represent a combination of SQL templates and their parameters. There are three types of commands: One, Many, and Exec, each with corresponding extension functions generated by KSP for use with QueryDsl.
I would greatly appreciate it if you could take a look at the PR and share your thoughts. Your feedback will be invaluable in ensuring that this feature meets the needs of the community and integrates well with the existing framework.

You can find the PR here.
https://github.com/komapper/komapper/pull/1346

Thank you in advance for your time and input!

[dave08]

Looks nice! I probably have to give it more thought, but right now, I don't really have too much time... I still dropped a little comment in the PR at least for now...

[dave08]

Kotlin isn't the limit there, like I showed in the new comment... I'm just wondering if KSP can process that val there.

[dave08]

Also, I think you'd also have to deal with singleOrNull()... sometimes I'd expect to have an answer, sometimes I need to know if it exists or not.

[dave08]

Also, what about Inserts with RETURNING...?

[Toshihiro Nakamura]

I think you’d also have to deal with singleOrNull()

We can write as follows:

class GetAuthor(val id: Int) : One<Author?>({ selectAsAuthor().singleOrNull() })

Also, what about Inserts with RETURNING...?

For now, the priority is low because the DSL already has a returning function. https://www.komapper.org/docs/reference/query/querydsl/insert/#returning
Do we really need RETURNING in SQL templates?

[dave08]

When there's complex sqls, it could very well come in handy... but I need to take a look at our projects to see if we actually use it in complex statements. I could try in the next few days...

[dave08]

Another little point w/o really having a chance to go in depth in your implementation is a very common case: pagination and ordering. Those are passed into most queries, and usually involve a bunch of boilerplate... it's something that's usually dynamically patched on at run time, how could this feature support this?

[dave08]

Not always would there be an offset or limit, and sometimes no order by, or sometimes different values for which fields to order by...

[dave08]

We usually have a data class Pagination(val from: Int, val limit: Int) that's nullable passed into queries and an enum for order by's. Could this support such parameters instead of having to make froms and limits and converting enums to strings everywhere?

[dave08]

Also, the template would have to always include those extra lines... maybe being able to derive a new annotation from @KomapperCommand @PaginatedCommand with the sql prefixed or suffixed with common sql? Maybe this is all overkill, I'm just comparing to what we currently do with other frameworks...

[Toshihiro Nakamura]

To construct dynamic queries, Komapper’s SQL templates support conditional branching, loops, and string interpolation. Are you saying that these features do not meet your needs? Could you provide a sample code to illustrate your use case?

[dave08]

In most of our queries we need to tag in LIMIT ? OFFSET ? conditionally, so we have util functions that do that for all queries that need it (and they automatically add the parameters to the parameter list). with the current templating feature, that code could also be appended to the string, but if the template is in an annotation it would have to be repeated for all our queries (and class params in One... etc..) ... same with order by.

[Toshihiro Nakamura]

I think embedded variable directives could be useful in your use case. However, if you prefer to continue with your current approach (automatic string concatenation and parameter addition), it might be best to keep using template queries.
Queries using @KomapperCommand are just an optional approach.

[dave08]

Maybe adding that to the Command class?

@KomapperCommand("""
select name, age from person where age > 1 /*# orderBy */
""")
class GetPerson(orderBy: EmbeddedDirective): Many<Person>

[Toshihiro Nakamura]

Yes, that’s correct. The type of the orderBy variable can be anything.

[dave08]

But here it could be better protected at compile time:

@KomapperEmbeddedDirective("""LIMIT /*limit*/ OFFSET /*offset*/""")
class Pagination(val limit: Int, val offset: Int)

@KomapperCommand("""
select name, age from person where age > 1 /*# pagination */
""")
class GetPerson(pagination: Pagination?): Many<Person>

would that be possible?

[Toshihiro Nakamura]

It’s an interesting idea, but we may need to consider whether it’s truly useful. As you might already know, the following is currently possible:

class Pagination(val limit: Int, val offset: Int)

@KomapperCommand("""
select name, age from person where age > 1 
/*%if pagination != null */
LIMIT /* pagination.limit */ OFFSET /*pagination.offset*/
/*%end*/
""")
class GetPerson(pagination: Pagination?): Many<Person>

[dave08]

Yes, but that has to be repeated TONS of times... it's something sooo common in apis to require pagination for just everything. So imagine copying and pasting that 50 times, and how error-prone that could get...

[dave08]

There might also be other things like JOINS that re-occur over lots of sql statements... it could be nice to embed them in a safe way that the sql is checked as valid at compile-time

[dave08]

Maybe all that's really needed is a proper way to inject that

/*%if pagination != null */
LIMIT /* pagination.limit */ OFFSET /*pagination.offset*/
/*%end*/

in the @KomapperCommand 's sql somehow... with a regular template query it's just a string, here it's an annotation... I wonder if this would work:

const val paginationTemplate = """
/*%if pagination != null */
LIMIT /* pagination.limit */ OFFSET /*pagination.offset*/
/*%end*/
"""

@KomapperCommand("""
select name, age from person where age > 1 
$paginationTemplate
""")
class GetPerson(pagination: Pagination?): Many<Person>

[Toshihiro Nakamura]

Hi, I have implemented RETURNING support and partial SQL fragments. Since KSP cannot access const values, I introduced the @KomapperPartial annotation.

[dave08]

Perfect 👌🏼, that's great, thanks!

[dave08]

Just wondering though, do commands support putting fields into an ORDER BY dynamically now, I don't think we discussed that fully... as far as I understand, I'd have do use a not-safe embedded directive, will a command support that being passed in the command constructor?

[dave08]

It seems like all the fields passed into the command object are just binded into the ? prepared statements... so that wouldn't allow putting a field name there to use in an ORDER BY?

[Toshihiro Nakamura]

Yes, commands can accept parts of a dynamic query, such as the ORDER BY clause, through its constructor.

For example, you can write as follows:

@KomapperCommand(
    """
    select * from address where street = /*street*/'test' /*# orderBy */
    """,
)
class GetByStreet(val street: String, val orderBy: String) : One<Address>({ select(asAddress).single() })

val query = QueryDsl.execute(GetByStreet("STREET 10", "order by address_id"))

[dave08]

It just won't be checked at compile time... would partials maybe help here?

[dave08]

Say to ensure that I don't pass an incorrect ORDER BY statement, ORDER BY null which might happen... it would be nicer to have an order by partial and be able to just specify the fields being ordered by...

[dave08]

Of course, all the features you've already added are amazing, I'm just wondering if this might be just another small thing, and it for sure has a bunch of uses in practice.

[Toshihiro Nakamura]

Just to clarify, Komapper can only check two things at compile time: whether the syntax of the Komapper SQL template is correct and whether the parameter bindings are correct. It does not verify the correctness of the SQL syntax itself.
If you can determine the pattern for the ORDER BY clause in advance, you could represent it in a Partial like this:

/*%if orderByType == @example.OrderByType@A */
order by aaa
/*%elseif orderByType == @example.OrderByType@B */
order by aaa, bbb
/*%elseif orderByType == @example.OrderByType@C */
order by aaa, bbb, ccc
/*%end */

example.OrderByType is an enum type.

[dave08]

Interesting... that's an option! Though a bit verbose... but if used in enough places, it might be worth it!

[dave08]

Btw, wondering how complex it would be to add a @Language("KomapperTemplate") to Intellij... it seems like the syntax is complex enough that having a bit of highlighting and checking in the editor might be worth it.