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

DDL tests for simulated DBI #284

Closed
krlmlr opened this issue Apr 14, 2019 · 3 comments · Fixed by #499
Closed

DDL tests for simulated DBI #284

krlmlr opened this issue Apr 14, 2019 · 3 comments · Fixed by #499
Labels
feature a feature request or enhancement verb trans 🤖 Translation of dplyr verbs to SQL
Milestone

Comments

@krlmlr
Copy link
Member

krlmlr commented Apr 14, 2019

Methods like db_compute() and db_copy_to() currently require a real database to be tested against. Suggesting the following approach:

  1. Define a db_execute() generic, implement it only for the "TestConnection" class (which would print the SQL) and for "DBIConnection" (which forwards to DBI::dbExecute())
  2. Replace all calls to DBI::dbExecute() with calls to db_execute()
  3. Run tests with simulate_dbi(...), capture output

Reference: https://github.com/tidyverse/dbplyr/pull/280/files#r275091349

@hadley
Copy link
Member

hadley commented Apr 16, 2019

If we going to rearchitect stuff, I think it would be better to extract out a separate method that just generates the SQL.

@hadley hadley added feature a feature request or enhancement verb trans 🤖 Translation of dplyr verbs to SQL labels Apr 16, 2019
@hadley hadley added this to the 2.0.0 milestone Sep 17, 2020
@hadley
Copy link
Member

hadley commented Sep 22, 2020

Looks like these are the main generics that generate then execute SQL:

  • db_explain
  • db_analyze
  • db_save_query
  • db_create_index — but only used by MySQL and now I don't see why*
  • db_drop_table

I'm going to add matching sql_*() generics, and change the default db_*() to call use sql_* + dbExecute().

@hadley
Copy link
Member

hadley commented Sep 23, 2020

db_save_query() is a little trickier because it has to return the name, and MS SQL needs to mangle it. So probably needs another new generic. (Which I think SAP HANA will also use)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature a feature request or enhancement verb trans 🤖 Translation of dplyr verbs to SQL
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants