r-dbi / backends

Details on DBI backends
https://www.r-dbi.org/backends/
15 stars 7 forks source link

ClickHouseHTTP backend #75

Closed patzaw closed 1 year ago

patzaw commented 2 years ago

Hi

Because we needed secure connection to our ClickHouse instances and since it is not yet supported by RClickhouse, I've developed a ClickHouse R DBI based on ClickHouse HTTP interface. It supports HTTP and HTTPS connections (with or without ssl peer verification).

It's available on CRAN (https://cran.r-project.org/package=ClickHouseHTTP) and github (https://github.com/patzaw/ClickHouseHTTP).

It is slower than RClickhouse (mainly because of data format transformation). Thus, if you don't need SSL, I recommend to still use RClickhouse package.

I hope it helps anyway

krlmlr commented 2 years ago

Thanks. I wonder why it's not picked up by the synchronization, need to take a look.

krlmlr commented 2 years ago

I believe this is due to the usage of methods::setMethod() for declaring dbSendQuery(). This is the relevant code search used here:

https://github.com/search?q=org%3Acran%20setMethod%20dbSendQuery&type=code

It is found when I explicitly search for "methods::setMethod" instead of "setMethod":

https://github.com/search?q=org%3Acran+methods%3A%3AsetMethod+dbSendQuery&type=code

Is there any specific reason to explicitly qualifying the setMethod() call?

patzaw commented 2 years ago

It's a habit I have taken after past experiences with package dependencies. But I think, I can safely remove the explicit reference to the 'methods' library in this case if it makes your life easier when searching code.

krlmlr commented 1 year ago

I've tried various search queries for finding your package and also the others, none seem to work. I could use two or more GitHub searches, this would complicate the code a lot. Because a naked setMethod() is pretty standard, I'd like to ask you to make this change.

patzaw commented 1 year ago

I've removed the explicit path to {methods} function. I'll submit the new version to CRAN probably in January or February. Thanks for your feedback

patzaw commented 1 year ago

Hi @krlmlr A new version of the package is now available on CRAN with naked calls to setMethod() (https://github.com/cran/ClickHouseHTTP/blob/c4036bba72499954c914b71b1e495cc71a08f78d/R/connection.R). However, it seems that it is still not identified by your robot. This query does not seem to pick it up: https://github.com/search?o=desc&q=org%3Acran+setMethod+dbSendQuery&s=indexed&type=Code But if I remove the filter on "cran" org, it works (the package appears among the first matches in cran org): https://github.com/search?o=desc&q=setMethod+dbSendQuery&s=indexed&type=Code

krlmlr commented 1 year ago

Thanks, weird. I can see it when I access this link:

https://github.com/search?o=desc&q=org%3Acran+setMethod+dbSendQuery&s=indexed&type=Code

Should we give it a few more days?

patzaw commented 1 year ago

Sure. No hurry on my side. Thanks