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

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

Open
frevib opened this issue Jun 12, 2024 · 6 comments
Labels

Comments

@frevib
Copy link
Contributor

frevib commented Jun 12, 2024

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

@frevib frevib added the bug label Jun 12, 2024
@griffio
Copy link
Contributor

griffio commented Jun 12, 2024

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

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

@frevib
Copy link
Contributor Author

frevib commented Jun 12, 2024

@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
Copy link
Contributor

griffio commented Jun 13, 2024

@frevib issue has come up before #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

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:

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
Copy link
Contributor

griffio commented Jun 13, 2024

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

🔨 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
Copy link
Contributor Author

frevib commented Jun 13, 2024

Earlier I was looking at:

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
Copy link
Contributor Author

frevib commented Jun 14, 2024

fix #5303

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

2 participants