Skip to content

317 bigquery using dbi#328

Open
javier-gracia-tabuenca-tuni wants to merge 9 commits intoOHDSI:developfrom
javier-gracia-tabuenca-tuni:317_bigquery_using_DBI
Open

317 bigquery using dbi#328
javier-gracia-tabuenca-tuni wants to merge 9 commits intoOHDSI:developfrom
javier-gracia-tabuenca-tuni:317_bigquery_using_DBI

Conversation

@javier-gracia-tabuenca-tuni

Before you do a pull request, you should always file an issue and make sure the package maintainer agrees that it’s a problem, and is happy with your basic proposal for fixing it. We don’t want you to spend a bunch of time on something that we don’t think is a good idea.

Additional requirements for pull requests:

  • Adhere to the Developer Guidelines as well as the OHDSI Code Style.

  • If possible, add unit tests for new functionality you add.

  • Restrict your pull request to solving the issue at hand. Do not try to 'improve' parts of the code that are not related to the issue. If you feel other parts of the code need better organization, create a separate issue for that.

  • Make sure you pass R check without errors and warnings before submitting.

  • Always target the develop branch, and make sure you are up-to-date with the develop branch.

…s delayIfNecessaryForDdl and delayIfNecessaryForInsert, just like it does for JDBC
On the above delay function I had to fix the regex so that It works also when using .
If there is an error and dbms is 'bigquery' take the error message from err$body, that contains the real sql error
@javier-gracia-tabuenca-tuni
Copy link
Author

javier-gracia-tabuenca-tuni commented Mar 10, 2026

this PR is for #317

Changes:

  • ListTables.R: if dbms is 'bigquery' uses bigrquery:: function to list tables
  • InsertTable.R: if dbms is 'bigquery' and if dropTableIfExists deletes table before inserting, bigrquery:: is not doing that correctly
  • DBI.R: for a DatabaseConnectorDbiConnection if dbms is 'bigquery' uses delayIfNecessaryForDdl and delayIfNecessaryForInsert, just like it does for JDBC
  • Sql.R: On the above delay function I had to fix the regex so that It works also when using .
    If there is an error and dbms is 'bigquery' take the error message from err$body, that contains the real sql error

Minor Changes:

  • I added some skip to some test files. Tests assume to have a Postgress always set up which I dont have/needed only to test BQ. Please remove if needed.
  • in the test-InsertTable.R: removed the filter for Bigquery is slow (Without bulk upload), bcs now it is fast

Doubts I got when applying the tests. Not sure If changing the test is the correct path:

  • bigrquery doent return the columns in the same order that they have been inserted. I force the column order in the sql fetching the data
  • bigrquery doesn't insert time zones. I force the InserTable to remove that from the dataframe before insertion.
  • bigrquery lost precision when inserting floats. I changed the test to make an exception. Is that ok ??
  • dbplyrTestFunction failed at Test slicing. I changed the test to make an exception. Is that ok ??

Still TO DO
At the moment I haven't defaulted the DatabaseConnector to use DBI. I set that in the setup.R

Im not sure what would be the best way to modify the DatabaseConnector:::connectBigQuery function
Particularly how to pass the 'billing' parameter
And you prefer to have 'bigrquery::bq_auth' with in the DatabaseConnector:::connectBigQuery. bigrquery::bq_auth() use different parameters depending on the environment.

I can work on that if you believe this is going correctly

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 this pull request may close these issues.

1 participant