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

[Bug] Insert code of golang template seems to be incorrect #347

Open
newbeelearn opened this issue Feb 10, 2022 · 8 comments
Open

[Bug] Insert code of golang template seems to be incorrect #347

newbeelearn opened this issue Feb 10, 2022 · 8 comments

Comments

@newbeelearn
Copy link

newbeelearn commented Feb 10, 2022

https://github.com/xo/xo/blob/master/_examples/django/sqlite3/author.xo.go

// insert (primary key generated and returned by database)
const sqlstr = INSERT INTO authors ( +
author_id, name +
) VALUES ( +
$1, $2 +
)
// run
logf(sqlstr, a.Name)
res, err := db.ExecContext(ctx, sqlstr, a.AuthorID, a.Name)

In this code author_id should not be sent to database unless id's are getting maintained by the application which does not seems to be the case as later on it retrieves and sets the id

// retrieve id
id, err := res.LastInsertId()
if err != nil {
return logerror(err)
} // set primary key
a.AuthorID = int(id)

Earlier version of xo was producing correct code, i am not sure when was this issue introduced and how is this not caught in testing.

@hhhapz
Copy link
Contributor

hhhapz commented Feb 10, 2022

This is not an issue with the template. It seems as if the column is not being detected (more likely it wasn't being set) as autoincrement. All other examples handle this find. This does not impact anything but the example.

@newbeelearn
Copy link
Author

yes this is not issue with template rather it is issue with PK not getting detected. It impacts all sqlite databases as PK will not be detected. In my code i am using autoincrement and it is still inserting the id's.
as per documentation auto pk logic should work if either autoincrement or integer primary key is set.

SQLite Auto PK Logic

Checks the SQL that is used to generate the table contains the AUTOINCREMENT keyword.
Checks that the table was created with the primary key type of INTEGER.

@hhhapz
Copy link
Contributor

hhhapz commented Feb 11, 2022

The autoincrement logic works fine for a_bit_of_everything as well as the booktest examples. If you still have issues, please share your schema so we can take a look.

@newbeelearn
Copy link
Author

I did bit of digging and it looks like the sql generated by xo is not matching with that of schema however json of schema generated by xo is identifying the fields correctly.

here is the schema, generated schema and json

schema.sql.txt
xo.xo.json.txt
xo.xo.sql.txt

@hhhapz
Copy link
Contributor

hhhapz commented Feb 11, 2022

The cause of the errors is the comments within the table declaration, and their interference with the string manipulation to determine which column is autoincrement. This is necessary because sqlite does not provide a mechanism to properly get the sequences of a table (at least I was unable to find any better solution at the time of writing the query). If possible, try to strip out the comments. I'll look into if I can fix my SQL query, but it's crafty, and frankly it's not really high priority at the moment.

@newbeelearn
Copy link
Author

Ok, i removed the comments and result is still the same.
Offtopic is there a way to go back to previous version or i need to rebuild the code myself? as previous version was generating the code correctly.

@hhhapz
Copy link
Contributor

hhhapz commented Feb 11, 2022

You can use go install with a specific commit sha to revert to an older version.

@newbeelearn
Copy link
Author

fyi sqlite pragma table_info() indicates if the field is pk. May be this can be used in addition to the current heuristic that is getting used to identify the primary table

sample output is shown below
sq:test.db=> PRAGMA table_info(users);
cid | name | type | notnull | dflt_value | pk
-----+-------------------+--------------+---------+-------------------+----
0 | user_id | INTEGER | 1 | | 1
1 | email | VARCHAR(255) | 1 | | 0
2 | password | VARCHAR(255) | 1 | | 0
3 | created_at | TIMESTAMP | 1 | CURRENT_TIMESTAMP | 0
4 | username | VARCHAR(255) | 1 | | 0
5 | last_password_set | TIMESTAMP | 0 | | 0
6 | last_login | TIMESTAMP | 0 | CURRENT_TIMESTAMP | 0
7 | role | VARCHAR(255) | 1 | | 0

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

No branches or pull requests

2 participants