Feature/independent column name encoding #845
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hadley
approved these changes
Sep 13, 2024
| @@ -48,6 +48,10 @@ setMethod("show", "OdbcDriver", | |||
| #' not using UTF-8 you will need to set the encoding to get accurate | |||
| #' re-encoding. See [iconvlist()] for a complete list of available encodings | |||
| #' on your system. Note strings are always returned `UTF-8` encoded. | |||
| #' @param columnNameEncoding The text encoding for column names used on the | |||
| #' Database. May be different than the `encoding` argument. Defaults to | |||
| #' empty string which is equivalent to returning the column names without | |||
There was a problem hiding this comment.
I'm slightly surprised that this doesn't default to encoding. Are the column names and values encoded differently that often?
There was a problem hiding this comment.
Hi Hadley:
The issue that precipitated this PR is an instance where the content is encoded latin1 but the names are UTF-8. Prior to 1.5.0 the names were never de/coded regardless how how the encoding argument was specified, so in a sense we are just reverting to prior behavior, while retaining the flexibility that @shrektan added in 1.5.0.
|
I think this is good to merge? |
|
Thumbs up from me! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This closes #834.
Some history:
encodingargument todbConnect.latin1/8-bit ascii, for example, but the DB driver returns the column names usingUTF-8.To handle this, in this PR:
dbConnect. New argument iscolumnNameEncoding.To do before merging: