API refactor + Oracle fixes #577
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
Compare upstream: nanodbc/nanodbc@0fe6c73
Use as appropriately ( Viewer.R ). Also: - Cleanup odbcConnectionColumns: - All S4 methods funnel to a single implementation point. - No more usage of connection_sql_columns ( outside of odbcConnectionColumns ) - Viewer.R: - Remove unnecessary dbListTables call in actions - list_catalogs is wrapped in tryCatch since not all back-ends support catalogs and some ( oracle ) throws an exception/error.
hadley
approved these changes
Jun 9, 2023
R/Viewer.R
Outdated
| @@ -346,12 +347,12 @@ odbcConnectionActions.default <- function(connection) { | |||
| tableName <- dbQuoteIdentifier(connection, firstTable$table_name) | |||
|
|
|||
| # add schema | |||
| if (!is.null(firstTable$table_schema) && nchar(firstTable$table_schema) > 0) { | |||
| if (!is.null(firstTable$table_schema) && !is.na(firstTable$table_schema) && nchar(firstTable$table_schema) > 0) { | |||
There was a problem hiding this comment.
This level of type flexibility always makes me a bit nervous. Could we add some type checks higher up so we only need to check for one empty sentinel?
There was a problem hiding this comment.
Thanks Hadley:
As best as I can tell, the is.null(...) that was there to begin-with is not guarding anything; I think only the is.na is actually effective.
Kind of hesitant to remove the is.null since I am not sure why it was there in the first place. But at the same time this is not in any critical code-path; we can revisit if removing it breaks something.
Co-authored-by: Hadley Wickham <hadley@posit.co>
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.
Hi:
Fixes #575 , #540, #554
Some highlights:
Previously we had runaway usage of
connection_sql_tables, which accommodates "magic" parameter ( combinations ). This makes it difficult to customize parts of its functionality for individual back-ends. Now some of the magic use cases are split out intoodbcConnectionCatalogs,odbcConnectionSchemas, andodbcConnectionTableTypes.Specialize some of these for Oracle so that (hopefully) all of these are more performant ( and functional ): listing tables, writing to tables, and Rstudio/Viewer operations.