cashapp / sqldelight

SQLDelight - Generates typesafe Kotlin APIs from SQL
https://cashapp.github.io/sqldelight/
Apache License 2.0
6.01k stars 501 forks source link

Keywords as column name - generated code missing double quotes for INSERT statement #5298

Open frevib opened 2 weeks ago

frevib commented 2 weeks ago

SQLDelight Version

2.0.2

SQLDelight Dialect

Postgresql

Describe the Bug

Given a column name that is a keyword in Postgresql:

CREATE TABLE "order"
(
    id uuid PRIMARY KEY NOT NULL
);

and .sq file:

saveOrder:
INSERT INTO "order"
VALUES ?;

The following code is generated:

public fun saveGroup(group: Group) {
    driver.execute(-119_365_208, """
        |INSERT INTO order (id)
        |VALUES (?)
        """.trimMargin(), 1) {
          check(this is JdbcPreparedStatement)
          bindObject(0, groupAdapter.idAdapter.encode(group.id))
        }
    notifyQueries(-119_365_208) { emit ->
      emit("group")
    }
  }

This results in

ERROR: syntax error at or near "order"
  Position: 13

which is because of order is missing double quotes: "order".

The code for SELECT queries does include the double quotes.

Stacktrace

No response

griffio commented 2 weeks ago

Can confirm - it's specifically using the VALUES ? special SqlDelight syntax

It should work withINSERT INTO "order" (id) VALUES (?);

frevib commented 2 weeks ago

@griffio thx, that will work for now.

But then you would have to call saveOrder with many parameterssaveOrder(id, a, b, c, ...) instead of just saveOrder(order).

I wouldn't mind fixing this bug myself, but it's not trivial to get into Sqldelight with all its lexing, parsing, PSI etc. Are there some docs to learn how the code works?

griffio commented 2 weeks ago

@frevib issue has come up before https://github.com/cashapp/sqldelight/issues/4877 and is not specific to Postgresql Dialect

For fixing bugs, only some basic overview is given here https://github.com/cashapp/sqldelight/blob/6fa1522ca3e87b8e1379891a7d30422489198753/CONTRIBUTING.md - sadly it requires lots of trial and error to implement somethings.

An entry point to investigating issues is to start by adding a test that can be debugged here https://github.com/cashapp/sqldelight/blob/6fa1522ca3e87b8e1379891a7d30422489198753/sqldelight-compiler/src/test/kotlin/app/cash/sqldelight/core/tables/InterfaceGeneration.kt#L24

e.g Add a test to allow debugging

  @Test fun `issue 5298`() {
    val file = FixtureCompiler.compileSql(
      """
      |CREATE TABLE "order" (
      |  data_id INTEGER NOT NULL
      |);
      |selectForId:
      |INSERT INTO "order" VALUES ?;
      """.trimMargin(),
      tempFolder,
      overrideDialect = PostgreSqlDialect()
    )

    val column = file.compiledFile
    println(column)
  }

🎯 Place your debugger in some places: https://github.com/cashapp/sqldelight/blob/6fa1522ca3e87b8e1379891a7d30422489198753/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/util/TreeUtil.kt#L182

https://github.com/cashapp/sqldelight/blob/6fa1522ca3e87b8e1379891a7d30422489198753/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/util/TreeUtil.kt#L236

You can see the difference if you use INSERT INTO "order" (data_id) VALUES (?) vs INSERT INTO "order" VALUES ? the table name from the sq file is being replaced with order

Have a look and see if you want to take a try at it?

griffio commented 2 weeks ago

Another issue is that sometimes the problem is located in the SqlPsi project

e.g This problem could be here ??? -> https://github.com/AlecKazakova/sql-psi/blob/9d51d00e622afe3cc06c722f9cf8181b89b8dd30/core/src/main/kotlin/com/alecstrong/sql/psi/core/psi/SqlNamedElementImpl.kt#L25

The table name is being trimmed from ""order"" to "order"

TreeUtil is using the sqlpsi SqlNamedElementImpl

https://github.com/cashapp/sqldelight/blob/6fa1522ca3e87b8e1379891a7d30422489198753/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/util/TreeUtil.kt#L186

🔨 One potential quick fix is to use tableName.node.text instead of using the text accessor e.g prefix = "${parent!!.tableName.node.text} to get the unmodified table name

frevib commented 2 weeks ago

Earlier I was looking at:

https://github.com/cashapp/sqldelight/blob/6fa1522ca3e87b8e1379891a7d30422489198753/sqldelight-compiler/src/main/kotlin/app/cash/sqldelight/core/lang/util/TreeUtil.kt#L236

but the replacements seems to contain (?) most of the time when debugging the unit tests.

For SELECT queries the generated code is correct, so we could have look why that flow is producing correct results.

I will have a look and see how far I can come with this issue.

frevib commented 2 weeks ago

fix #5303