zjffdu commented on a change in pull request #3694: [ZEPPELIN-2891]. Impossible
to use jdbc interface with presto-jdbc >=0.180
URL: https://github.com/apache/zeppelin/pull/3694#discussion_r395424593
##########
File path: jdbc/src/main/java/org/apache/zeppelin/jdbc/JDBCInterpreter.java
##########
@@ -407,7 +408,33 @@ private void setUserProperty(String propertyKey,
InterpreterContext interpreterC
}
private void createConnectionPool(String url, String user, String
propertyKey,
- Properties properties) throws SQLException, ClassNotFoundException {
+ Properties properties) throws SQLException, ClassNotFoundException,
IOException {
+
+ String driverClass = properties.getProperty(DRIVER_KEY);
+ if (driverClass != null &&
(driverClass.equals("com.facebook.presto.jdbc.PrestoDriver")
+ || driverClass.equals("io.prestosql.jdbc.PrestoDriver"))) {
+ Properties newProperties = new Properties();
+ // Only add valid properties otherwise presto won't work.
+ try {
+ Class clazz = null;
+ try {
+ clazz =
Class.forName("com.facebook.presto.jdbc.ConnectionProperties");
+ } catch (ClassNotFoundException e) {
+ clazz = Class.forName("io.prestosql.jdbc.ConnectionProperties");
+ }
+ Method lookUpKeyMethod = clazz.getMethod("forKey", String.class);
Review comment:
@elec
> Please don’t access internals of the driver with reflection. We can change
this at any time in the future which will cause this code to break.
>
> You can find the list of supported properties here:
https://prestosql.io/docs/current/installation/jdbc.html
>
> The important thing is to not add random properties like “url”. Zeppelin
should not be adding properties unless it knows they are valid or they come
from an end user. (If the user adds an invalid property, that’s on the user,
and we don’t need to protect them from themselves.)
One concern I have is whether presto would change these valid properties.
Because we would like to support multiple versions of presto. So if presto
introduce new properties or remove some existing properties, that would be a
problem for us.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
