-
-
Notifications
You must be signed in to change notification settings - Fork 154
Add support for mixed case column names in PostgreSQL #166
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
Add support for mixed case column names in PostgreSQL #166
Conversation
@@ -73,7 +73,7 @@ open class SqlFormatter( | |||
} | |||
} | |||
|
|||
protected val String.quoted: String get() { | |||
open protected val String.quoted: String get() { | |||
if (this.toUpperCase() in database.keywords || !this.isIdentifier) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure what isIdentifier
signifies - while I don't believe there are any cases where quoting the column name in PostgreSQL will result in invalid syntax, there may be an edge case of which I'm unaware.
@@ -47,6 +47,10 @@ open class PostgreSqlFormatter(database: Database, beautifySql: Boolean, indentS | |||
return result | |||
} | |||
|
|||
override val String.quoted: String get() { | |||
return "${database.identifierQuoteString}${this}${database.identifierQuoteString}".trim() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am quoting all column names. I believe quoting all columns for consistency is the right approach, but additional logic could be added to only quote column names which contain one or more uppercase characters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for finding the bug and fixing it. Your solution is great. Always quoting the column names is the way that never get wrong.
I will merge your code. But before I merge that, please also update the build.gradle
file, add your GitHub ID to the developers info (line 88).
Awesome - done! |
Thanks for fixing my typo in |
I'm building a small service using an existing PostgreSQL database, and came across a bug where column names defined with mixed case (e.g. "columnName") did not generate the correct SQL for PostgreSQL - the column names were not quoted.
PostgreSQL transforms unquoted column names to their lowercase equivalent:
columnName
becomescolumnname
, which is not defined as a column.I resolved this by quoting all column names in the PostgreSQL dialect. To do this, I had to open the implementation of
String.quoted
inSqlDialect
. I could have done it by overridingPostgreSqlDialect.visitColumn
andPostgreSqlDialect.visitInsert
, but that seemed both unnecessarily verbose and duplicated much of the logic from the parent class.Note that I am very new to Kotlin, so please take care to ensure that my changes are appropriate.