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

dbAppendTable() no longer works with Id #380

Closed
renkun-ken opened this issue Feb 28, 2022 · 5 comments · Fixed by #381
Closed

dbAppendTable() no longer works with Id #380

renkun-ken opened this issue Feb 28, 2022 · 5 comments · Fixed by #381
Labels

Comments

@renkun-ken
Copy link
Contributor

renkun-ken commented Feb 28, 2022

See #253

In previous releases, dbAppendTable(con, Id(schema = "foo", table = "bar"), data) works as documented at https://github.com/r-dbi/DBI/blob/main/R/Id.R#L16 but has stopped working since v1.1.2.

The check at

stopifnot(is.character(name), length(name) == 1)

ignores that name might be Id and sqlAppendTableTemplate would call dbQuoteIdentifier with the name anyway.

table <- dbQuoteIdentifier(con, table)

Does it make sense if we remove the check of name in sqlAppendTableTemplate or allow it to be Id?

@krlmlr krlmlr added the bug label Feb 28, 2022
@krlmlr
Copy link
Member

krlmlr commented Feb 28, 2022

Thanks, makes sense. We also need a test in DBItest.

@renkun-ken
Copy link
Contributor Author

Do you prefer simply removing the check or allowing it to be Id in the check?

@krlmlr
Copy link
Member

krlmlr commented Feb 28, 2022

Should we check after calling dbQuoteIdentifier() ?

@renkun-ken
Copy link
Contributor Author

Looks like dbQuoteIdentifier already does all necessary checks.

@github-actions
Copy link
Contributor

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue and link to this old issue if necessary.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants