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

Loss of names in dbUnquoteIdentifier()/too specific test #339

Closed
nbenn opened this issue Dec 22, 2023 · 1 comment · Fixed by #341
Closed

Loss of names in dbUnquoteIdentifier()/too specific test #339

nbenn opened this issue Dec 22, 2023 · 1 comment · Fixed by #341

Comments

@nbenn
Copy link
Member

nbenn commented Dec 22, 2023

I'm sorry, I could not make the reprex more self-contained than the following:

  1. take r-dbi/adbi@623dda0 together with current CRAN DBItest
  2. remove the skip "list_objects_features" (at https://github.com/r-dbi/adbi/blob/623dda0236616ed9a9cc64a878ed222479faecda/tests/testthat/test-DBItest.R#L47)
  3. enjoy a single test failure as
    ── Failed tests ──────────────────────────────────────────────────────────────────────────
    Failure (/private/var/folders/k4/0jwzxmln0nb8y6rkzprptb640000gq/T/RtmpydzGY4/R.INSTALL1523a5e94a720/DBItest/R/run.R:77:11): DBItest[adbi]: SQL: list_objects_features
    `unquoted` not equal to unclass(objects$table[!objects$is_prefix]).
    Component 1: Attributes: < Component 2: Names: 1 string mismatch >
    Component 2: Attributes: < Component 2: Names: 1 string mismatch >
    Backtrace:
        ▆
     1. ├─rlang::exec(test_fun, !!!args) at DBItest/R/run.R:77:10
     2. └─DBItest (local) `<fn>`(ctx = `<DBItst_c>`, con = `<AdbCnnct>`)
     3.   ├─testthat::expect_warning(...) at DBItest/R/spec-sql-list-objects.R:131:4
     4.   │ └─testthat:::quasi_capture(...)
     5.   │   ├─testthat (local) .capture(...)
     6.   │   │ └─base::withCallingHandlers(...)
     7.   │   └─rlang::eval_bare(quo_get_expr(.quo), quo_get_env(.quo))
     8.   └─testthat::expect_equal(unquoted, unclass(objects$table[!objects$is_prefix]))
    

The comparison that fails is

expect_equal(unquoted, unclass(objects$table[!objects$is_prefix]))

where we have

Browse[1]> str(unquoted)
List of 2
 $ :Formal class 'Id' [package "DBI"] with 1 slot
  .. ..@ name: Named chr "dbitdrizyuhylp"
  .. .. ..- attr(*, "names")= chr ""
 $ :Formal class 'Id' [package "DBI"] with 1 slot
  .. ..@ name: Named chr "dbitintzpndbpf"
  .. .. ..- attr(*, "names")= chr ""
Browse[1]> str(unclass(objects$table[!objects$is_prefix]))
List of 2
 $ :Formal class 'Id' [package "DBI"] with 1 slot
  .. ..@ name: Named chr "dbitdrizyuhylp"
  .. .. ..- attr(*, "names")= chr "table"
 $ :Formal class 'Id' [package "DBI"] with 1 slot
  .. ..@ name: Named chr "dbitintzpndbpf"
  .. .. ..- attr(*, "names")= chr "table"

Note that objects is created by dbListObjects() and unquoted is created more or less as dbUnquoteIdentifier(dbQuoteIdentifier(.)). Somewhere in this quote/unquote operation (adbi relies on DBI for that), the Id names are lost. A small example for this is

x <- Id(table = "test")
str(x)
#> Formal class 'Id' [package "DBI"] with 1 slot
#>   ..@ name: Named chr "test"
#>   .. ..- attr(*, "names")= chr "table"
str(DBI::dbUnquoteIdentifier(DBI::ANSI(), DBI::dbQuoteIdentifier(DBI::ANSI(), x)))
#> List of 1
#>  $ :Formal class 'Id' [package "DBI"] with 1 slot
#>   .. ..@ name: Named chr "test"
#>   .. .. ..- attr(*, "names")= chr ""

So I guess what I'm trying to say is that I believe it is DBI-provided functionality that causes my test failure and either the test should be relaxed (ignore name attributes) or DBI quote/unquote should preserve names.

(I believe the reproducibility issue comes from the fact, that the test is not self-contained as the DB state that is required for failure is not established as part of the test itself, but in some preceding test that I could not find.)

@krlmlr
Copy link
Member

krlmlr commented Dec 22, 2023

Thanks. Yes, this looks like fallout from the changes to Id() . Agree to make the test less specific, I opened a PR. Does it work for you?

library(DBI)

x <- Id(table = "test")

xx <- dbUnquoteIdentifier(ANSI(), dbQuoteIdentifier(ANSI(), x))

dput(x)
#> new("Id", name = c(table = "test"))
dput(xx[[1]])
#> new("Id", name = structure("test", names = ""))

x <- Id("test")

xx <- dbUnquoteIdentifier(ANSI(), dbQuoteIdentifier(ANSI(), x))

dput(x)
#> new("Id", name = structure("test", names = ""))
dput(xx[[1]])
#> new("Id", name = structure("test", names = ""))

Created on 2023-12-22 with reprex v2.0.2

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

Successfully merging a pull request may close this issue.

2 participants