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

Use double quotes when referencing database objects #792

Closed
12cowdog72 opened this issue Jan 14, 2022 · 6 comments · Fixed by #1059
Closed

Use double quotes when referencing database objects #792

12cowdog72 opened this issue Jan 14, 2022 · 6 comments · Fixed by #1059

Comments

@12cowdog72
Copy link

Related to #739

Postgres allows certain reserved keywords and special characters (question marks, for example) to be used in table/column/etc. names, but they must be escaped in double quotes.

The following migration:

create table_for(Mail) do
  add from : String
  add read? : Bool
end

ideally should generate the following SQL:

create table "mails" (
  "from" text,
  "read?" boolean
);

but since it does not add double quotes, the generated SQL statements will cause a syntax error in Postgres. This also happens in queries and such which could make it impossible to use Avram with certain existing databases without renaming every offending field.

@jwoertink
Copy link
Member

I had no idea that you could add characters to table/column names... That's a pretty neat use case for Bool; however, I feel a bit hesitant about its use for anything else. This would allow someone to do add username? : String which would generate methods like UserQuery.new.username?("..."), or make passing in named args to save operations like SaveUser.create("username?": "...").

I feel like using this could lead to both bad code, and lots of weird edge cases we'd have to consider. It may just be easier to disallow using any non-alphanumeric character for these. Though, you may have a database that already has a column like this, in which case we need to implement #378. I wonder if Ecto is able to handle this? 🤔

Thanks for bringing this up! Definitely something new to consider!

@robacarp
Copy link
Contributor

The decision of using punctuation in column names aren't always up to the app developer -- sometimes the database is inherited. Regardless, I think it would be prudent to reference column names with quotes regardless of the decision to let people use punctuation in their column names.

@jwoertink
Copy link
Member

Yeah, that makes sense. I guess we can start there. That will at least fix some of the SQL issues, and doesn't really hurt anything for simple DBs.

@matthewmcgarvey
Copy link
Member

matthewmcgarvey commented Jan 15, 2022

I think we already have a way to specify a column name that's different from the model's attribute, and same for the table name, so there is workarounds. We just need to also quote it, I guess

@jwoertink
Copy link
Member

This just came up in discord again. Having a column named order throws an error, so it would need to be escaped. Anyone know how bad it would be to just do double quotes around ALL of them always? Or is this something we would need to be configurable on a per column basis?

@robacarp
Copy link
Contributor

robacarp commented Jul 5, 2024

I believe rails and ecto both always wrap column names in quotes, always.

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.

4 participants