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

[Bugfix] Enable creation of tables whose fieldName otherwise clashes with SQL keywords such as group #165

Open
PhilippMDoerner opened this issue Aug 14, 2022 · 1 comment

Comments

@PhilippMDoerner
Copy link
Collaborator

In the nim discord ajusa previously encountered this issue where they had a database, but one of the columns they had was called group.
This causes issues with sqlite's SQL (and likely postgres but I didn't test that). In the language you can avoid those by escaping the name with '.

So minimal reproducible example:

import norm/[model, sqlite]

type A = ref object of Model
    group: string 

let x = A(group: "")
let con = open(":memory:", "", "", "")
con.createTables(x)
Error: unhandled exception: near "group": syntax error [DbError]

The SQL generated is

CREATE TABLE IF NOT EXISTS "A"(group TEXT NOT NULL, id INTEGER NOT NULL PRIMARY KEY)

To fix this, the SQL it would need to generate would be this (can also use " instead of '):

CREATE TABLE IF NOT EXISTS "A"('group' TEXT NOT NULL, id INTEGER NOT NULL PRIMARY KEY)

A similar issue goes for selectAll as it generates for example this sql:

import norm/[model, sqlite]

type A* = ref object of Model
    group*: string 

let con = open(":memory:", "", "", "")

let y = """CREATE TABLE IF NOT EXISTS "A"('group' TEXT NOT NULL, id INTEGER NOT NULL PRIMARY KEY)"""
con.exec(sql y)

var objs = @[A(group: "")]
con.selectAll(objs)
SELECT "A".group, "A".id FROM "A"  WHERE 1

Should be generating

SELECT "A"."group", "A"."id" FROM "A"  WHERE 1
@moigagoo moigagoo linked a pull request Aug 15, 2022 that will close this issue
@moigagoo
Copy link
Owner

There's an ungoing PR that fixes this by adding quotes around column names.

The PR stuck on test suite update: since the table names are all different now, the entire test suite needs revision and update.

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 a pull request may close this issue.

2 participants