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

Enable additional JVM parameters on JDBC() instantiation #42

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hjort
Copy link

@hjort hjort commented Mar 7, 2017

When additional JVM parameters must be passed during the database connection, JDBC() class might accept an additional argument.

For instance, in order to connect to a Teiid data source constrained by SSL security requirements, one must specify the private JKS key through -Djavax.net.ssl.trustStore argument.

With the current pull request, RJDBC is able to accept JVM additional arguments seamlessly, as it was already available in rJava package.

Here is an example of that functionality:

library(RJDBC)

drv<-JDBC('org.teiid.jdbc.TeiidDriver', '/opt/jdbc/teiid-8.7.1.redhat-8-jdbc.jar', '"', '-Djavax.net.ssl.trustStore=/opt/jdbc/truststore-pro-2014.jks')

@kafran
Copy link

kafran commented Apr 25, 2019

Will this be merged?

@hjort
Copy link
Author

hjort commented Apr 29, 2019

Well, I don't know. It's been a while.

@s-u
Copy link
Owner

s-u commented Apr 29, 2019

Well, it's not that simple. Unlike that classpath, there is no guarantee that you can set the JVM parameters - if JVM is already initialized (either by another package or because you're embedded in running Java) it's a no-op so the patch certainly isn't complete. Part of the issue that we're now going quite low-level - if this is about Java properties then you can set them in the runtime as well. So I think this is simply the wrong approach. If I read the original issue correctly, it may make more sense to allow setting Java properties (which can be done at runtime) instead of JVM startup-flags.

@s-u s-u force-pushed the master branch 5 times, most recently from 30654b9 to 7675ca0 Compare March 12, 2021 02:41
@tsykes
Copy link

tsykes commented Mar 24, 2021

I came across this as I was trying to set a truststore on my RJDBC connection. Here's how I ended up doing it:

# install.packages("RJDBC")
props <-  c("-Djavax.net.ssl.trustStore=C:\\windows\\path\\to\\truststore.jks",
            "-Djavax.net.ssl.trustStorePassword=changeit",
            "-Djavax.net.ssl.trustStoreType=jks",
            "-Doracle.net.ssl_version=1.2")

options(java.parameters = props) 

library(RJDBC)

drv <- JDBC("oracle.jdbc.driver.OracleDriver","C:\\windows\\path\\to\\oracle\\driver\\ojdbc8-12.2.0.1.jar")

url <- "jdbc:oracle:thin:@(DESCRIPTION=(ADDRESS=(PROTOCOL=tcps)(HOST=servername) PORT=2494))(CONNECT_DATA=(SERVICE_NAME=servicename)))"

conn <- dbConnect(drv, url, "user", "password")

chk <- dbGetQuery(conn, "SELECT 5432 betterport FROM DUAL ") 

chk

@s-u
Copy link
Owner

s-u commented Mar 24, 2021

@tsykes yes, that only works if no other package has started Java before.

What I was saying is that it may be better to set properties instead as they can be set even in a running JVM. So what I had in mind was something like:

setProps <- function(...) {
   l=list(...)
   for (k in names(l))
      J("java.lang.System")$setProperty(k, l[[k]])
}

setProps(
        oracle.net.ssl_version="1.2",
        javax.net.ssl.trustStoreType="jks",
        javax.net.ssl.trustStorePassword="changeit",
        javax.net.ssl.trustStore="C:\\windows\\path\\to\\truststore.jks")

Does that work for you? That's what I was suggesting so if it works for you that's easy to add...

Perhaps to clarify: if this is about run-time Java properties that's easy to implement, but that's not what this PR was addressing. This PR was about setting JVM start-up arguments which are a different beast since you can't change them once the JVM is running (e.g. the stack, heap etc.), so the approach in this PR doesn't work. If setting run-time properties is sufficient for the examples people use then that's a different, but much easier issue.

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.

4 participants