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

fix: EXPOSED-244 [PostgreSQL] Collate option on column not recognized #1956

Merged
merged 1 commit into from
Jan 3, 2024

Conversation

bog-walk
Copy link
Member

@bog-walk bog-walk commented Jan 2, 2024

Unlike all other dialects, PostgreSQL column COLLATE clause requires that the option is quoted as part of the column definition.

Unlike all other dialects, PostgreSQL column COLLATE clause requires that the
option is quoted as part of the column definition.
@bog-walk bog-walk requested a review from e5l January 2, 2024 21:09
Comment on lines 547 to +553
protected fun escape(value: String): String = value.map { charactersToEscape[it] ?: it }.joinToString("")

/** Returns the specified [value] with special characters escaped and wrapped in quotations, if necessary. */
protected fun escapeAndQuote(value: String): String = when (currentDialect) {
is PostgreSQLDialect -> "\"${escape(value)}\""
else -> escape(value)
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This logic could not be applied directly to escape() because nonNullValueToString() is dependent on it.

The quotation is only necessary in sqlType() overrides.

Copy link
Member

@e5l e5l left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@bog-walk bog-walk merged commit 330eadf into main Jan 3, 2024
5 checks passed
@bog-walk bog-walk deleted the bog-walk/fix-string-collate branch January 3, 2024 18:12
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 this pull request may close these issues.

2 participants