New odbc method for sql.reader

[tomschloss]

Relates to Issue #144

Pulls through the default options for RODBC::odbcDriverConnect using formals. Args specified in the .sql doc (with type: odbc) are compared to the formals list. Those that are RODBC::odbcDriverConnect args replace the defaults (with check.else.default), while those that aren't (excluding type and query) are assumed to be values in the connection string. These remaining args are concatenated to form the connection string with separated.list.

I've had to specify defaults for colQuote and tabQuote. They were set to double quotes as suggested in the RODBC documentation, although this can be changed in the .sql file with colQuote: and tabQuote: if required.

There is probably a more elegant way to do this, but this should be compatible with .sql files made in the old world (although this hasn't been tested).

RStudio and roxygen2 went a bit gangbusters with documentation updates. Let me know if that is an issue...

Main changes:
sql.reader

    # list of rodbc options to ignore
    odbcDC_default_args <-
      as.list(formals(RODBC::odbcDriverConnect))
    arg_names <- formalArgs(RODBC::odbcDriverConnect)

    # colQuote default
    odbcDC_default_args[['colQuote']] <- '\"'
    odbcDC_default_args[['tabQuote']] <- odbcDC_default_args[['colQuote']]

    # define args: each element is an arg from odbcDriverConnect, and takes the
    # value of the default, unless specified in database.info
    odbcDC_args <-
      lapply(
        names(odbcDC_default_args),
        check.else.default,
        default = odbcDC_default_args,
        options = database.info
      )
    names(odbcDC_args) <- names(odbcDC_default_args)

    # create connection.string from args that are not odbcDriverConnect
    # or query args
    connection.string <-
      separated.list(
        target.list = database.info,
        ignore = c(arg_names, 'type', 'query'),
        sepchar = ';',
        colchar = '='
      )
    odbcDC_args[['connection']] <- connection.string

    # open the connection
    connection <- do.call(RODBC::odbcDriverConnect, odbcDC_args)
    results <- RODBC::sqlQuery(connection, database.info[['query']])
    RODBC::odbcClose(connection)

[KentonWhite]

Thanks so much for this PR. I'm really looking forward to reviewing it and will clear some time over the net few days!

[KentonWhite]

Does changing dbname to database break backward compatibility?

[tomschloss]

I think on reflection it does. It will also have an impact on the user, password and dsn fields. I'll fix this.

[KentonWhite]

I really like querying the driver for its arguments and defaults. Really we should think about moving all of the connections to this method in the future.

For maintenance, my biggest concern is backward compatibility with the change of dbname to database. While the ODBC driver expects a database argument this might just break some projects. Perhaps adding a method to set database to dbname if dbname is given plus a deprecation warning?

To help our future selves, can you add a test illustrating using the new parameters for ODBC please. While there are existing tests for the ODBC drivers, they only exercise the old parameters and defaults. Adding a new test ensures that future changes to the code or to the ODBC drivers don't break the feature.

Finally, can you squash into a single commit please. I appreciate having the commit history while working on the feature branch is invaluable, once merged into master it can make the master history difficult to read.

Thanks for this PR -- it looks really great!

[tomschloss]

Thanks for looking over it - much appreciated. I'll have some time later this week incorporate your comments

[KentonWhite]

Closing for now since it breaks backward compatibility. Please re-open if the backward compatibility issue is solved or there is a pressing need to break backward compatibility.