Proposal: quote column names #14

New Issue

lunny · 2018-04-26T19:50:28Z

lunny commented

2018-04-26 19:50:28 +00:00

Author: @fasterthanlime

Currently, builder never quotes anything:

[10.515µs] SELECT profile_id,game_id,order FROM profile_games WHERE profile_id=? [389]

SQLITE_ERROR: near "order": syntax error (SELECT profile_id,game_id,order FROM profile_games WHERE profile_id=?)

Would you be open to making it escape columns and tables, so that instead of:

SELECT profile_id,game_id,order FROM profile_games WHERE profile_id=?

It would generate:

SELECT "profile_id","game_id","order" FROM "profile_games" WHERE "profile_id"=?

(I'm using builder with SQLite, but I believe at least MySQL & PostgreSQL both support escaping colum and table names with double quotes ("))

As usual I can work on a PR as soon as I get your 👍

Author: @fasterthanlime Currently, builder never quotes anything: ``` [10.515µs] SELECT profile_id,game_id,order FROM profile_games WHERE profile_id=? [389] SQLITE_ERROR: near "order": syntax error (SELECT profile_id,game_id,order FROM profile_games WHERE profile_id=?) ``` Would you be open to making it escape columns and tables, so that instead of: ``` SELECT profile_id,game_id,order FROM profile_games WHERE profile_id=? ``` It would generate: ``` SELECT "profile_id","game_id","order" FROM "profile_games" WHERE "profile_id"=? ``` (I'm using builder with SQLite, but I believe at least MySQL & PostgreSQL both support escaping colum and table names with double quotes (`"`)) As usual I can work on a PR as soon as I get your :+1:

lunny commented

2018-04-26 21:40:42 +00:00

Author: @fasterthanlime

Mhh maybe builder is not the right place to do this. Maybe the ORM is the right place to do this. How does xorm handle this?

Author: @fasterthanlime Mhh maybe builder is not the right place to do this. Maybe the ORM is the right place to do this. How does xorm handle this?

lunny commented

2018-04-27 00:55:34 +00:00

Author: @lunny

@fasterthanlime xorm will quote the columns generated by struct. I agree builder could have an optional to add quote to all columns. But to keep compatible, the default will not add quote automatically. And in fact you could add quote manually at present. For example:

Select("profile_id", "`order`")...

Author: @lunny @fasterthanlime xorm will quote the columns generated by struct. I agree builder could have an optional to add quote to all columns. But to keep compatible, the default will not add quote automatically. And in fact you could add quote manually at present. For example: ```Go Select("profile_id", "`order`")... ```

lunny commented

2018-04-27 07:11:17 +00:00

Author: @fasterthanlime

xorm will quote the columns generated by struct.

That makes sense, I'll make my persistent layer do the same. Thanks for the input, let's keep builder simple!

Author: @fasterthanlime > xorm will quote the columns generated by struct. That makes sense, I'll make my persistent layer do the same. Thanks for the input, let's keep builder simple!

lunny commented

2018-06-17 09:45:53 +00:00

Author: @hurisheng

if using xorm, column names in 'select' is quoted, but condition fields is not quoted. I found that in xorm source code, for example, genGetSQL() generates condition with builder.ToSQL(statement.cond) which doesn't quote field name (https://github.com/go-xorm/xorm/blob/master/statement.go#L962). The rest part of genGetSQL() generate select statement with Quote() (https://github.com/go-xorm/xorm/blob/master/statement.go#L1030). So the final SQL would looks like (mysql):

SELECT `id`, `name`, `group` FROM `user` WHERE group=? LIMIT 1

As group is reserved word in mysql, so the result is syntax error.
Of course we can quote manually, but I think if xorm can quote the condition or builder supports quote is better way for consistency. Quoted is always better than not.
@lunny Could you please give some thoughts how to implement this, I would like to try create PR.

Author: @hurisheng if using xorm, column names in 'select' is quoted, but condition fields is not quoted. I found that in xorm source code, for example, `genGetSQL()` generates condition with `builder.ToSQL(statement.cond)` which doesn't quote field name (https://github.com/go-xorm/xorm/blob/master/statement.go#L962). The rest part of `genGetSQL()` generate select statement with `Quote()` (https://github.com/go-xorm/xorm/blob/master/statement.go#L1030). So the final SQL would looks like (mysql): ```sql SELECT `id`, `name`, `group` FROM `user` WHERE group=? LIMIT 1 ``` As `group` is reserved word in mysql, so the result is syntax error. Of course we can quote manually, but I think if xorm can quote the condition or builder supports quote is better way for consistency. Quoted is always better than not. @lunny Could you please give some thoughts how to implement this, I would like to try create PR.

lunny commented

2018-08-09 02:26:03 +00:00

Author: @lunny

@hurisheng You have to add quote in your self builder conditions. Quote feature should be implemented in future builder version.

Author: @lunny @hurisheng You have to add quote in your self builder conditions. Quote feature should be implemented in future builder version.

Sign in to join this conversation.