NyxCode / ormx

bringing orm-like features to sqlx
MIT License
284 stars 33 forks source link

Quote identifiers in generated queries #23

Open Punie opened 2 years ago

Punie commented 2 years ago

On my current project, our naming standards require our table names to be singular, which led to the following issue in the case of the user table:

#[derive(Table)]
#[ormx(table = "user", id = id)]
struct User {
    #[ormx(column = "id")]
    id: Uuid,
    email: String,
    password_hash: String,
}

yields the following errors:

error: error returned from database: column "id" does not exist
  --> server/src/user.rs:31:10
   |
31 | #[derive(Table)]
   |          ^^^^^
   |
   = note: this error originates in the macro `$crate::sqlx_macros::expand_query` (in Nightly builds, run with -Z macro-backtrace for more info)

error: error returned from database: syntax error at or near "user"
  --> server/src/user.rs:31:10
   |
31 | #[derive(Table)]
   |          ^^^^^
   |
   = note: this error originates in the macro `$crate::sqlx_macros::expand_query` (in Nightly builds, run with -Z macro-backtrace for more info)

The message is not ideal but I managed to figure out that it is indeed the fact that user is a reserved name and that it should be quoted. My current workaround (I'm using Postgres) is the following #[ormx(table = "\"user\"", id = id)] and it does work but I wonder if ormx should not defensively quote identifiers by default.

I understand that it requires custom logic from different backends (MySQL requires backticks `user` while Postgres requires double-quotes "user") so it is likely non-trivial to implement. At the very least, maybe warning about that potential issue in documentation and recommending quoting identifiers in the derive macro attributes would be the way to go in the short-term.

Punie commented 2 years ago

Oh my bad, I just realized that #19 already raises a similar issue and it made me realize that there is already some logic for reserved identifiers here.

I guess the only thing missing would then be to ignore the case of that list.