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

Found more than one class "Microsoft SQL Server" in cache; using the first, from namespace '.GlobalEnv' #224

Closed
ablack3 opened this issue Mar 3, 2023 · 9 comments

Comments

@ablack3
Copy link
Collaborator

ablack3 commented Mar 3, 2023

I'm seeing Found more than one class "Microsoft SQL Server" in cache; using the first, from namespace '.GlobalEnv' printed a whole lot.

I think this is because the "Microsoft SQL Server" class is defined in two different packages that I have installed, DatabaseConnector and odbc.

My sense is that it is not a good idea to export a class that is also exported by another package but I could be wrong.

Defintion in DatabaseConnector:

setClass("Microsoft SQL Server",

exportClasses("Microsoft SQL Server")

Defintion in odbc:
https://github.com/r-dbi/odbc/blob/7cdeac8af11b5d304493dcdda82c3d4f63ca3452/R/db.R#L98

Does not look like odbc exports this class though...
https://github.com/r-dbi/odbc/blob/main/NAMESPACE

All I know for sure is that this message is getting printed a lot.
image

@schuemie
Copy link
Member

schuemie commented Mar 6, 2023

Yes, I looked at this. Unfortunately, dbplyyr doesn't nicely export the class, it just creates it at runtime, so we can't inherit from that. I didn't see it in odbc, so maybe we can inherit from that. Ideally, there would be a central class that we'd all inherit from.

@ablack3
Copy link
Collaborator Author

ablack3 commented Mar 6, 2023

Ok just wanted to report this. I'm not sure if it causes any real problems. Feel free to close it if there's nothing to be done.

@ablack3
Copy link
Collaborator Author

ablack3 commented Jul 27, 2023

DatabaseConnector is overwriting another package's class if I'm understanding this error correctly. Just having DatabaseConnector installed is causing issues for users connecting to SQL Server with odbc.

image

@schuemie
Copy link
Member

schuemie commented Aug 9, 2023

Weird. I see the warning ("Found more than one class..."), but not the error message, and I can use the db object just fine.

I'll try to see if I can mimic odbc's trick of dynamically creating the class (as done here). Seems less nice than a global static class definition, but may solve this problem.

@schuemie
Copy link
Member

schuemie commented Aug 9, 2023

I have a first attempt in the dynamic_inheritance branch. The warning (and probably error) should be gone when using odbc.

The error does return when DatabaseConnector::connect() has been called prior to using odbc in which case the Microsoft SQL Server class has been dynamically defined by DatabaseConnector. Not yet sure what to do about that.

Could you give the current version in dynamic_inheritance a try? (Check is currently throwing some warnings about missing documentation, but should not prevent testing)

@ablack3
Copy link
Collaborator Author

ablack3 commented Aug 9, 2023

What if we tried adding a DatabaseConnector translation method to dbplyr? It could do the same thing as sql server but wouldn't overwrite the class.

The idea would be for dbplyr to generate generic OHDSI-SQL which would then get translated by sql render?

I'll try the dynamic inheritance branch.

@schuemie
Copy link
Member

schuemie commented Aug 9, 2023

I like the idea of adding a OhdsiSql backend to dbplyr. I'm also thinking about the future of DatabaseConnector in general. I would like it to become more and more lightweight, as the DBI drivers are becoming more and more mature and consistent with each other. DatabaseConnector should just be there to fix the last annoying incompatibility issues between drivers, not duplicate what is already there. So perhaps we may not need an OhdsiSql backend in the end (this dynamic inheritance might allow us to use the various native backends instead of SQL translation). Just thinking out loud...

@ablack3
Copy link
Collaborator Author

ablack3 commented Aug 9, 2023

Yea. we've had pretty good success using odbc in Darwin but there are still some annoying differences between databases. But overall it's good. I've had a lot of issues using odbc on Oracle so it would be nice if we could use DatabaseConnector's DBI interface for Oracle and maybe snowflake as well. Basically users can just plug in whatever DBI driver user's want to use including but not limited to DatabaseConnector.

Probably one of the biggest differences between the dbplyr translation and Sqlrender has been quoting of identifiers in SQL.

Sqlrender has to quote at least one cdm column...
image

I've been testing cross platform code a lot lately so getting some good experience with that. Not so easy! 😄

@ablack3
Copy link
Collaborator Author

ablack3 commented Aug 9, 2023

This issue seems to be fixed in the dynamic_inheritance branch. Thank you!!

library(DatabaseConnector)
con <- DBI::dbConnect(odbc::odbc(),
                      Driver   = Sys.getenv("SQL_SERVER_DRIVER"),
                      Server   = Sys.getenv("CDM5_SQL_SERVER_SERVER"),
                      Database = Sys.getenv("CDM5_SQL_SERVER_CDM_DATABASE"),
                      UID      = Sys.getenv("CDM5_SQL_SERVER_USER"),
                      PWD      = Sys.getenv("CDM5_SQL_SERVER_PASSWORD"),
                      TrustServerCertificate="yes",
                      Port     = 1433)

DBI::dbDisconnect(con)

Created on 2023-08-09 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

No branches or pull requests

2 participants