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

Add support for naming tables with I() #708

Open
hadley opened this issue Dec 22, 2023 · 4 comments
Open

Add support for naming tables with I() #708

hadley opened this issue Dec 22, 2023 · 4 comments

Comments

@hadley
Copy link
Member

hadley commented Dec 22, 2023

And eliminate double dispatch for dbExistsTable() and friends

setMethod(
  "dbExistsTable", c("OdbcConnection"),
  function(conn, name, ...) {
    
    if (is(name, "SQL") || inherits(x, "AsIs")) {
      # converts to an Id object
      name <- dbUnquoteIdentifier(conn, name)[[1]]
    }
     
    if (is(name, "Id")) {
      df <- odbcConnectionTables(
        conn,
        name = id_field(name, "table"),
        catalog_name = id_field(name, "catalog"),
        schema_name = id_field(name, "schema")
      )
    } else if (is.character(name) && length(name) == 1) {
      df <- odbcConnectionTables(conn, name = name, ..., exact = TRUE)
    } else {
      abort("`name` must be a string, Id(), SQL() object.")
    }
    
    NROW(df) > 0
  }
)

Need to check if we need c("OdbcConnection", "ANY") in the signature to avoid ambiguous dispatch, and if we can make dbWriteTable() work the same way.

@simonpcouch
Copy link
Collaborator

Also seems worth checking for arguments that would be ignored, e.g. database = in the issue linked above.

@hadley
Copy link
Member Author

hadley commented Dec 22, 2023

@simonpcouch maaaaaybe. I don't think the named Id() idea is particularly well thought out so I'm not sure how much effort we want to put into mitigating using it incorrectly. But it's probably worth having something to check that the Id is well formed (i.e. it has two or three components).

@simonpcouch
Copy link
Collaborator

The fact that several folks came across #423 after its submission and a similar, well-voted-on SO Post that made the same mistake in its answer (I just submitted an edit) makes me feel like helping folks out with incorrect usage is worth the effort.

@hadley
Copy link
Member Author

hadley commented Dec 22, 2023

Ok, lets add some simple checks when we tackle this.

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

No branches or pull requests

2 participants