Update docs to prefer `I()` over `in_schema()` etc

[hadley]

• And make sure copy_to() and tbl() have good description of that argument
• Make in_schema() internal
• Make ident() internal

[MichaelChirico]

Could I understand the motivation for this? Honestly it seems pretty counterintuitive/obstructive.

• Basically every remote backend query I see will be in schema.table form, so this will create a lot of toil in required changes to existing call sites
• I fundamentally would expect I("schema.table") to be communicating "Use the single name schema.table, escaping . if necessary", not "Use the two-part name schema + table". My understanding of I() is usually "don't try to interpret this", which matches the former usage.

[hadley]

@MichaelChirico yes, in an ideal world we would have made schema.table the rule and a weirdly named table called schema.table the exception. Unfortunately that mistake was made a long time ago and is very hard to back out of, so this change just makes the best of a bad situation.

If you want a better mental model for I() think of it as opting-out of the normal escaping of ..

[MichaelChirico]

Could we perhaps opt in to the "old" way with an option? It'll be a fair amount of churn to support this & it's not easy to detect S3 stuff statically to help with migration. For now I am just editing our methods to continue supporting I()-less approach.

[hadley]

Maybe, but (a) I'm worried about more churn in the interface (and it's now been stable for a while) and (b) I just don't have the time to fully analyse and understand the consequences of such a change.

[MichaelChirico]

Thanks!

For any other readers maybe looking for a workaround, this patch to our methods seems to be serving our purposes well enough:

name <- maybe_asis(name, con)

maybe_asis <- function(x, con) {
  if (is.null(x) || inherits(x, c("AsIs", "dbplyr_table_path")) || isS4(x)) {
    return(x)
  }
  if (length(dbUnquoteIdentifier(con, x)[[1L]]@name) <= 1L) {
    return(x)
  }
  return(I(x))
}