unnamed Id() breaks dbQuoteIdentifier()

[dpprdan]

r-dbi/DBI#417, i.e. allowing unnamed components in Id(), breaks dbQuoteIdentifier() (dbQuoteIdentifier_PqConnection_Id specifically), because the latter currently relies (and checks for) on component names.

library(RPostgres)

conn <- postgresDefault()

dbQuoteIdentifier(conn, Id("mytable"))
#> Error in dbQuoteIdentifier(conn, Id("mytable")): all(names(x@name) %in% c("catalog", "schema", "table")) is not TRUE

dbDisconnect(conn)

Session info

sessioninfo::session_info()
#> ─ Session info ───────────────────────────────────────────────────────────────
#>  setting  value
#>  version  R version 4.3.2 (2023-10-31 ucrt)
#>  os       Windows 11 x64 (build 22631)
#>  system   x86_64, mingw32
#>  ui       RTerm
#>  language en
#>  collate  German_Germany.utf8
#>  ctype    German_Germany.utf8
#>  tz       Europe/Berlin
#>  date     2023-12-20
#>  pandoc   3.1.11 @ C:/PROGRA~1/Pandoc/ (via rmarkdown)
#> 
#> ─ Packages ───────────────────────────────────────────────────────────────────
#>  package     * version    date (UTC) lib source
#>  bit           4.0.5      2022-11-15 [1] CRAN (R 4.3.0)
#>  bit64         4.0.5      2020-08-30 [1] CRAN (R 4.3.0)
#>  blob          1.2.4      2023-03-17 [1] CRAN (R 4.3.0)
#>  cli           3.6.1      2023-03-23 [1] CRAN (R 4.3.0)
#>  DBI           1.1.3.9017 2023-12-20 [1] Github (r-dbi/DBI@62056e2)
#>  digest        0.6.33     2023-07-07 [1] CRAN (R 4.3.1)
#>  evaluate      0.22       2023-09-29 [1] CRAN (R 4.3.1)
#>  fastmap       1.1.1      2023-02-24 [1] CRAN (R 4.3.0)
#>  fs            1.6.3      2023-07-20 [1] CRAN (R 4.3.1)
#>  generics      0.1.3      2022-07-05 [1] CRAN (R 4.3.0)
#>  glue          1.6.2      2022-02-24 [1] CRAN (R 4.3.0)
#>  hms           1.1.3      2023-03-21 [1] CRAN (R 4.3.0)
#>  htmltools     0.5.6.1    2023-10-06 [1] CRAN (R 4.3.1)
#>  knitr         1.45       2023-10-30 [1] CRAN (R 4.3.1)
#>  lifecycle     1.0.3      2022-10-07 [1] CRAN (R 4.3.0)
#>  lubridate     1.9.3      2023-09-27 [1] CRAN (R 4.3.1)
#>  magrittr      2.0.3      2022-03-30 [1] CRAN (R 4.3.0)
#>  pkgconfig     2.0.3      2019-09-22 [1] CRAN (R 4.3.0)
#>  purrr         1.0.2      2023-08-10 [1] CRAN (R 4.3.1)
#>  R.cache       0.16.0     2022-07-21 [1] CRAN (R 4.3.0)
#>  R.methodsS3   1.8.2      2022-06-13 [1] CRAN (R 4.3.0)
#>  R.oo          1.25.0     2022-06-12 [1] CRAN (R 4.3.0)
#>  R.utils       2.12.2     2022-11-11 [1] CRAN (R 4.3.0)
#>  reprex        2.0.2      2022-08-17 [1] CRAN (R 4.3.0)
#>  rlang         1.1.1      2023-04-28 [1] CRAN (R 4.3.0)
#>  rmarkdown     2.25       2023-09-18 [1] CRAN (R 4.3.1)
#>  RPostgres   * 1.4.6      2023-10-22 [1] CRAN (R 4.3.1)
#>  rstudioapi    0.15.0     2023-07-07 [1] CRAN (R 4.3.1)
#>  sessioninfo   1.2.2      2021-12-06 [1] CRAN (R 4.3.0)
#>  styler        1.10.2     2023-08-29 [1] CRAN (R 4.3.1)
#>  timechange    0.2.0      2023-01-11 [1] CRAN (R 4.3.0)
#>  vctrs         0.6.4      2023-10-12 [1] CRAN (R 4.3.1)
#>  withr         2.5.2      2023-10-30 [1] CRAN (R 4.3.1)
#>  xfun          0.40       2023-08-09 [1] CRAN (R 4.3.1)
#>  yaml          2.3.7      2023-01-23 [1] CRAN (R 4.3.0)
#> 
#>  [1] C:/Users/Daniel.AK-HAMBURG/AppData/Local/R/win-library/4.3
#>  [2] C:/Program Files/R/R-4.3.2/library
#> 
#> ──────────────────────────────────────────────────────────────────────────────

[krlmlr]

Thanks for the heads-up. What do you mean by "breaks"? Is this different from "a feature that is not yet implemented here"?

[dpprdan]

RPostgres/NEWS.md

Line 287 in 73dbac8

- Names in the `x` argument to `dbQuoteIdentifier()` are preserved in the output (r-lib/DBI#173).

r-dbi/DBI#173

Not a contradiction per se, but 🤔

[dpprdan]

What do you mean by "breaks"? Is this different from "a feature that is not yet implemented here"?

Aren't all bugs just features not yet implemented? 😄

Just saw, so I guess: "Great minds..." 😉

[krlmlr]

The NEWS item you mentioned is not about component names, but about the names of the resulting vector.

[dpprdan]

The NEWS item you mentioned is not about component names, but about the names of the resulting vector.

Technically true, but don't the names for the resulting vector usually stem from the component names (if x is an Id())?

[krlmlr]

list(
  A = c(a = 1, b = 2),
  B = c(a = 3, c = 4)
)

This is about the outer names A and B, not the inner (component) names a and b.

[dpprdan]

What do you mean by "breaks"? Is this different from "a feature that is not yet implemented here"?

Sorry, I may have misunderstood. Did you mean whether I think this might become a bigger issue beyond implementing the change here?

Well, I was vaguely sceptical of the unnamed Id change at first, because Hadley's assertion "we never actually use them, never need to care about them" is obviously not true (see above - though replacing "never" with "rarely" makes it true again) and without the names it can be hard to tell which database structure a component is referring to. It is comparable to matching arguments by name and by position and we're essentially losing the option to match by name.
That said, it is rarely necessary to tell which database structure a component is referring to and it's still possible to order Id() components by name (including columns 🚀 ). So do I have a concrete example where something breaks beyond "not yet implemented"? No, I don't.

---

Anyway, re "not yet implemented": #372 should fix this issue now.

[dpprdan]

That said, it is rarely necessary to tell which database structure a component is referring to

Not so rarely after all:

RPostgres/R/tables.R

Line 135 in f15f8f4

if ("schema" %in% names(id)) {

RPostgres/R/tables.R

Line 170 in f15f8f4

table <- dbQuoteString(conn, id[["table"]])

RPostgres/R/dbListObjects_PqConnection_ANY.R

Lines 25 to 27 in f15f8f4

	"schema" %in% names(x@name) && !("table" %in% names(x@name))
	})
	schemas <- vcapply(prefix[is_prefix], function(x) x@name[["schema"]])

So right now I am more sceptical again, but I will have to look at this again with a clear head.

[dpprdan]

I assume r-dbi/DBI#422 will have to be implemented here as well, @krlmlr?

Do we still need this right now, if the user has a way to create Id objects without naming the components? Why is it important that dbUnquoteIdentifier() returns unnamed Id objects?

What was the answer to your question? Why would we throw away information?

RPostgres currently relies on this here

RPostgres/R/dbExistsTable_PqConnection_character.R

Line 8 in f15f8f4

id <- dbUnquoteIdentifier(conn, name)[[1]]@name

and here

RPostgres/R/dbListFields_PqConnection_character.R

Line 5 in f15f8f4

id <- dbUnquoteIdentifier(conn, quoted)[[1]]@name

and then in the places mentioned one post up ☝🏻.

(I also do in #413 and #414)

[krlmlr]

Dealt with most of the fallout. I suspect dbExistsTable(con, Id(...)) doesn't currently work.

Issues for tests: r-dbi/DBItest#340, r-dbi/DBItest#367.

Please open a new issue linking here if needed.