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

Quote identifiers in generated queries #23

Closed
Punie opened this issue Jan 11, 2022 · 2 comments
Closed

Quote identifiers in generated queries #23

Punie opened this issue Jan 11, 2022 · 2 comments

Comments

@Punie
Copy link
Contributor

Punie commented Jan 11, 2022

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

Punie commented Jan 11, 2022

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.

@NyxCode
Copy link
Owner

NyxCode commented Sep 8, 2024

fixed in #40

@NyxCode NyxCode closed this as completed Sep 8, 2024
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