This is an automated email from the ASF dual-hosted git repository.
chengpan pushed a commit to branch branch-1.6
in repository https://gitbox.apache.org/repos/asf/kyuubi.git
The following commit(s) were added to refs/heads/branch-1.6 by this push:
new 7f2f5a4d2 [KYUUBI #4311] Fix the wrong parsing of jvm parameters in
jdbc url.
7f2f5a4d2 is described below
commit 7f2f5a4d20c58db4a47b92e75fd0df6d2c121940
Author: wxmimperio <[email protected]>
AuthorDate: Wed Feb 15 11:04:08 2023 +0000
[KYUUBI #4311] Fix the wrong parsing of jvm parameters in jdbc url.
### _Why are the changes needed?_
The wrong parsing of jvm parameters in jdbc url.
For example:
```shell
jdbc:kyuubi://127.0.0.1:2181/;serviceDiscoveryMode=zooKeeper;zooKeeperNamespace=kyuubi;user=tom?spark.driver.memory=8G;spark.driver.extraJavaOptions=-Xss256m
-XX:+PrintGCDetails -XX:HeapDumpPath=/heap.hprof
```
Now because the regex `([^;]*)=([^;]*);?` only matches `=`, resulting in
wrong parsing:
```shell
<spark.driver.memory, 8G>
<spark.driver.extraJavaOptions=-Xss256m -XX:+PrintGCDetails
-XX:HeapDumpPath, /heap.hprof>
```
The correct parsing should be:
```shell
<spark.driver.memory, 8G>
<spark.driver.extraJavaOptions, -Xss256m -XX:+PrintGCDetails
-XX:HeapDumpPath=/heap.hprof>
```
This PR change `org.apache.kyuubi.jdbc.hive.Utils` and added unit test in
`org.apache.kyuubi.jdbc.hive.UtilsTest`.
### _How was this patch tested?_
- [x] Add some test cases that check the changes thoroughly including
negative and positive cases if possible
- [ ] Add screenshots for manual tests if appropriate
- [x] [Run
test](https://kyuubi.readthedocs.io/en/master/develop_tools/testing.html#running-tests)
locally before make a pull request
Closes #4311 from imperio-wxm/master.
Closes #4311
d9e124ca [liangbowen] fix percent encoding problem
31ae63e0 [liangbowen] change `com.sun.jndi.toolkit.url.UrlUtil` to
`java.net.URLEncoder`
f8271ec2 [wxm] Fix the wrong parsing of jvm parameters in jdbc url.
2e93df57 [wxmimperio] Merge branch 'apache:master' into master
4c236447 [wxmimperio] Merge branch 'apache:master' into master
3e0b7764 [wxmimperio] Scala repl output log level adjusted to debug
Lead-authored-by: wxmimperio <[email protected]>
Co-authored-by: liangbowen <[email protected]>
Co-authored-by: wxm <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
(cherry picked from commit b79ca8c4ab26d826a87599c0599f4d124754ce4b)
Signed-off-by: Cheng Pan <[email protected]>
---
.../java/org/apache/kyuubi/jdbc/hive/Utils.java | 13 +++-
.../org/apache/kyuubi/jdbc/hive/UtilsTest.java | 75 ++++++++++++++++++++--
2 files changed, 79 insertions(+), 9 deletions(-)
diff --git
a/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/Utils.java
b/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/Utils.java
index 886daeb71..7afdf261e 100644
--- a/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/Utils.java
+++ b/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/Utils.java
@@ -26,6 +26,7 @@ import java.sql.SQLException;
import java.util.*;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
+import org.apache.commons.lang3.StringUtils;
import org.apache.hive.service.rpc.thrift.TStatus;
import org.apache.hive.service.rpc.thrift.TStatusCode;
import org.slf4j.Logger;
@@ -190,12 +191,20 @@ public class Utils {
}
}
+ Pattern confPattern = Pattern.compile("([^;]*)([^;]*);?");
+
// parse hive conf settings
String confStr = jdbcURI.getQuery();
if (confStr != null) {
- Matcher confMatcher = pattern.matcher(confStr);
+ Matcher confMatcher = confPattern.matcher(confStr);
while (confMatcher.find()) {
- connParams.getHiveConfs().put(confMatcher.group(1),
confMatcher.group(2));
+ String connParam = confMatcher.group(1);
+ if (StringUtils.isNotBlank(connParam) && connParam.contains("=")) {
+ int symbolIndex = connParam.indexOf('=');
+ connParams
+ .getHiveConfs()
+ .put(connParam.substring(0, symbolIndex),
connParam.substring(symbolIndex + 1));
+ }
}
}
diff --git
a/kyuubi-hive-jdbc/src/test/java/org/apache/kyuubi/jdbc/hive/UtilsTest.java
b/kyuubi-hive-jdbc/src/test/java/org/apache/kyuubi/jdbc/hive/UtilsTest.java
index c890c8731..e0594dde0 100644
--- a/kyuubi-hive-jdbc/src/test/java/org/apache/kyuubi/jdbc/hive/UtilsTest.java
+++ b/kyuubi-hive-jdbc/src/test/java/org/apache/kyuubi/jdbc/hive/UtilsTest.java
@@ -21,8 +21,13 @@ package org.apache.kyuubi.jdbc.hive;
import static org.apache.kyuubi.jdbc.hive.Utils.extractURLComponents;
import static org.junit.Assert.assertEquals;
+import com.google.common.collect.ImmutableMap;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Map;
import java.util.Properties;
import org.junit.Test;
import org.junit.runner.RunWith;
@@ -35,23 +40,76 @@ public class UtilsTest {
private String expectedPort;
private String expectedCatalog;
private String expectedDb;
+ private Map<String, String> expectedHiveConf;
private String uri;
@Parameterized.Parameters
- public static Collection<String[]> data() {
+ public static Collection<Object[]> data() throws
UnsupportedEncodingException {
return Arrays.asList(
- new String[][] {
- {"localhost", "10009", null, "db",
"jdbc:hive2:///db;k1=v1?k2=v2#k3=v3"},
- {"localhost", "10009", null, "default", "jdbc:hive2:///"},
- {"localhost", "10009", null, "default", "jdbc:kyuubi://"},
- {"localhost", "10009", null, "default", "jdbc:hive2://"},
- {"hostname", "10018", null, "db",
"jdbc:hive2://hostname:10018/db;k1=v1?k2=v2#k3=v3"},
+ new Object[][] {
+ {
+ "localhost",
+ "10009",
+ null,
+ "db",
+ new ImmutableMap.Builder<String, String>().put("k2", "v2").build(),
+ "jdbc:hive2:///db;k1=v1?k2=v2#k3=v3"
+ },
+ {
+ "localhost",
+ "10009",
+ null,
+ "default",
+ new ImmutableMap.Builder<String, String>().build(),
+ "jdbc:hive2:///"
+ },
+ {
+ "localhost",
+ "10009",
+ null,
+ "default",
+ new ImmutableMap.Builder<String, String>().build(),
+ "jdbc:kyuubi://"
+ },
+ {
+ "localhost",
+ "10009",
+ null,
+ "default",
+ new ImmutableMap.Builder<String, String>().build(),
+ "jdbc:hive2://"
+ },
+ {
+ "hostname",
+ "10018",
+ null,
+ "db",
+ new ImmutableMap.Builder<String, String>().put("k2", "v2").build(),
+ "jdbc:hive2://hostname:10018/db;k1=v1?k2=v2#k3=v3"
+ },
{
"hostname",
"10018",
"catalog",
"db",
+ new ImmutableMap.Builder<String, String>().put("k2", "v2").build(),
"jdbc:hive2://hostname:10018/catalog/db;k1=v1?k2=v2#k3=v3"
+ },
+ {
+ "hostname",
+ "10018",
+ "catalog",
+ "db",
+ new ImmutableMap.Builder<String, String>()
+ .put("k2", "v2")
+ .put("k3", "-Xmx2g -XX:+PrintGCDetails
-XX:HeapDumpPath=/heap.hprof")
+ .build(),
+ "jdbc:hive2://hostname:10018/catalog/db;k1=v1?"
+ + URLEncoder.encode(
+ "k2=v2;k3=-Xmx2g -XX:+PrintGCDetails
-XX:HeapDumpPath=/heap.hprof",
+ StandardCharsets.UTF_8.toString())
+ .replaceAll("\\+", "%20")
+ + "#k4=v4"
}
});
}
@@ -61,11 +119,13 @@ public class UtilsTest {
String expectedPort,
String expectedCatalog,
String expectedDb,
+ Map<String, String> expectedHiveConf,
String uri) {
this.expectedHost = expectedHost;
this.expectedPort = expectedPort;
this.expectedCatalog = expectedCatalog;
this.expectedDb = expectedDb;
+ this.expectedHiveConf = expectedHiveConf;
this.uri = uri;
}
@@ -76,5 +136,6 @@ public class UtilsTest {
assertEquals(Integer.parseInt(expectedPort),
jdbcConnectionParams1.getPort());
assertEquals(expectedCatalog, jdbcConnectionParams1.getCatalogName());
assertEquals(expectedDb, jdbcConnectionParams1.getDbName());
+ assertEquals(expectedHiveConf, jdbcConnectionParams1.getHiveConfs());
}
}
