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

The 'statement' argument to dbGetQuery and dbSendQuery methods isn't exposed in older versions of R #170

Closed
tyner opened this issue Nov 4, 2021 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@tyner
Copy link

tyner commented Nov 4, 2021

Issue Description

The 'statement' argument to dbGetQuery and dbSendQuery methods isn't exposed in older versions of R, resulting in an error that it cannot find the 'statement' object.

Reproducible Example

First, install version 2.3.0 of the noctua package under R version 3.4.4. Then take a look at

library(noctua)
showMethods("dbGetQuery", includeDefs=TRUE)
showMethods("dbSendQuery", includeDefs=TRUE)

It will say:

Function: dbGetQuery (package DBI)
conn="AthenaConnection", statement="character"
function (conn, ...)
{
    .local <- function (conn, statement = NULL, statistics = FALSE,
        unload = FALSE, ...)
    {
        con_error_msg(conn, msg = "Connection already closed.")
        stopifnot(is.logical(statistics), is.logical(unload))
        rs <- dbSendQuery(conn, statement = statement, unload = unload)
        if (statistics)
            print(dbStatistics(rs))
        out <- dbFetch(res = rs, n = -1, ...)
        dbClearResult(rs)
        return(out)
    }
    .local(conn, statement, ...)
}

and

Function: dbSendQuery (package DBI)
conn="AthenaConnection", statement="character"
function (conn, ...)
{
    .local <- function (conn, statement = NULL, unload = FALSE,
        ...)
    {
        con_error_msg(conn, msg = "Connection already closed.")
        stopifnot(is.logical(unload))
        res <- AthenaResult(conn = conn, statement = statement,
            s3_staging_dir = conn@info$s3_staging, unload = unload)
        return(res)
    }
    .local(conn, statement, ...)
}

Whereas, under R version 3.5.0 (and later), the methods do expose the statement argument, and there is no error when using them. So naturally I am wondering if this is due to R-core correcting a bug going from version 3.4.4 to 3.5.0 of R, or could it be considered a bug in noctua itself? If the former, should noctua require R version >= 3.5.0? In any case, curious to hear your thoughts or suggestions.

Session Info
devtools::session_info()
#> output
Session info -------------------------------------------------------------------
 setting  value                       
 version  R version 3.4.4 (2018-03-15)
 system   x86_64, linux-gnu           
 ui       X11                         
 language (EN)                        
 collate  en_US.UTF-8                 
 tz       America/New_York            
 date     2021-11-04                  

Packages -----------------------------------------------------------------------
 package    * version date       source        
 data.table   1.12.6  2019-10-18 CRAN (R 3.4.4)
 DBI          1.1.0   2019-12-15 CRAN (R 3.4.4)
 devtools     1.12.0  2016-12-05 CRAN (R 3.4.0)
 digest       0.6.27  2020-10-24 CRAN (R 3.4.4)
 memoise      1.1.0   2017-04-21 CRAN (R 3.4.0)
 noctua     * 2.3.0   2021-10-26 CRAN (R 3.4.4)
 paws         0.1.12  2021-09-03 CRAN (R 3.4.1)
 rstudioapi   0.11    2020-02-07 CRAN (R 3.4.4)
 withr        2.1.2   2018-03-15 CRAN (R 3.4.4)
@DyfanJones
Copy link
Owner

Hi @tyner, thanks for identify this issue. I will look into this to see if it is a bug within the package.

@DyfanJones
Copy link
Owner

@tyner just added a fix to the current PR branch bump-dev. I have tested with R-3.3.3 so I am hoping it works for you 😄 Please let me know.

remotes::install_github("dyfanjones/noctua", ref="bump-dev")

@DyfanJones DyfanJones added the bug Something isn't working label Nov 4, 2021
@DyfanJones DyfanJones self-assigned this Nov 4, 2021
@tyner
Copy link
Author

tyner commented Nov 4, 2021

Thanks for the quick turnaround! I ran into a bit of a snag when it asked about upgrading xml2. I tried two different ways and got the same error each time. I also tried upgrading to the latest version of withr, which didn't resolve the issue. Alternatively, is there a way to directly download a .tar.gz version of the updated noctua package?

remotes::install_github("dyfanjones/noctua", ref="bump-dev")
Downloading GitHub repo dyfanjones/noctua@bump-dev
These packages have more recent versions available.
It is recommended to update all of them.
Which would you like to update?

1: All                               
2: CRAN packages only                
3: None                              
4: xml2 (268387bb1... -> NA) [GitHub]

Enter one or more numbers, or an empty line to skip updates: 3
Error: Failed to install 'noctua' from GitHub:
  'local_makevars' is not an exported object from 'namespace:withr'
remotes::install_github("dyfanjones/noctua", ref="bump-dev")
Downloading GitHub repo dyfanjones/noctua@bump-dev
These packages have more recent versions available.
It is recommended to update all of them.
Which would you like to update?

1: All                               
2: CRAN packages only                
3: None                              
4: xml2 (268387bb1... -> NA) [GitHub]

Enter one or more numbers, or an empty line to skip updates: 1
xml2 (268387bb1... -> NA) [GitHub]
Downloading GitHub repo hadley/xml2@master
Error: Failed to install 'noctua' from GitHub:
  Failed to install 'xml2' from GitHub:
  'local_makevars' is not an exported object from 'namespace:withr'

@DyfanJones
Copy link
Owner

Intersting, remotes install worked for me. However if you have all the dependencies you can try

remotes::install_github("dyfanjones/noctua", ref="bump-dev", dependencies = FALSE)

Note if you don't have the dependencies this will fail.

@tyner
Copy link
Author

tyner commented Nov 5, 2021

That did the trick. I have tested the new version of noctua under a version of R prior to 3.5.0 and it works splendidly.

I am still curious to learn if R-core had intentionally made setMethod more robust in this regard for version 3.5.0, or if the observed behavior was a happy accident associated with some other change. I may inquire about that on the mailing list.

In any case, thank you Dyfan!

@DyfanJones
Copy link
Owner

No worries, I will push these changes to cran next week. If you need them sooner please let me know.

DyfanJones added a commit to DyfanJones/RAthena that referenced this issue Nov 12, 2021
@DyfanJones
Copy link
Owner

This has now been pushed to the cran in release 2.4.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants