dbWriteTable and dbReadTable generics

Hi,

I do agree that having row.names=, append=, and overwrite= makes sense,
because these arguments are common and quite natural to most current DBI
method implementations of these generic functions.  However, adding them
explicitly to the generic definition implies that methods can be dispatched
on the *class* of those same arguments, which I'm not clear whether it's
helpful.

Here, I seem to see two similar but not-quite equal objectives:

(1) defining an interface more strictly (i.e., the DBI dictates that
dbWriteTables, etc., must have a more complete signature including
row.names=, etc.). Perfectly reasonable, yet setMethod for the various
implementations may (?) need to be revised (if so, not a big a deal).

(2) defining the signature on which methods are be dispatched.

I don't think that (1) is wrong (after all, the current use of name= is
precisely this case) or that (1) and (2) are mutually exclusive. However,
the original intention was clearly (2): methods are to be dispatched only
on the conn= and name= arguments, other arguments (...) may be passed from
the generic to the method(s), but not used for dispatching.  I still feel
that the current definition expresses this intention most clearly and
unambigiously.  Adding row.names=, etc., for the sake of a more complete
interface may imply behavior we (I)  haven't anticipated, e.g., what error
do users get when they incorrectly supply "row.names = t" (t as the
transpose as opposed to T for TRUE)?  Overall, not a particular big issue.

My $0.02

Regards,

David

On Fri, Oct 18, 2013 at 9:56 AM, Hadley Wickham <h.wickham at gmail.com>
wrote:

I'd like to discuss one small change that will require changes in some
dependencies: updating the arguments to dbWriteTable and dbReadTable.

snip

I'd like to move these to the generics to get:

setGeneric("dbWriteTable",
  def = function(conn, name, value, row.names = FALSE,
                       overwrite = FALSE, append = FALSE, ...) {
     standardGeneric("dbWriteTable")
  }, valueClass = "logical"
)

setGeneric("dbReadTable",
  def = function(conn, name, row.names = FALSE, ...) {
    standardGeneric("dbReadTable")
  },
  valueClass = "data.frame"
)

I'd also suggest a change to the semantics of row.names. It should be:

* TRUE to use the row.names column
* FALSE to not use row names
* a character string to specify an alternative column to use for row

names

I think this is close to the currently convention, but it is not well
documented.

This will require a small amount of work for any maintainers who
currently don't implement these options.

I'll also be careful when handling any future cran releases of DBI -
all downstream maintainers will get a heads up at least a month before
submission before CRAN so you'll have time to make any changes without
cran breathing down your neck.

+1 on these from me.

+ seth


--
Seth Falcon | @sfalcon | http://userprimary.net/

        [[alternative HTML version deleted]]

_______________________________________________
R-sig-DB mailing list -- R Special Interest Group
R-sig-DB at r-project.org
https://stat.ethz.ch/mailman/listinfo/r-sig-db