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.

Sign up for GitHub

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

Fix the wrong parsing of jvm parameters in jdbc URL #11

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

Fix the wrong parsing of jvm parameters in jdbc URL #11

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b27c140
Fix the wrong parsing of jvm parameters in jdbc URL
yanghua Apr 14, 2023
26aa92a
[KYUUBI #4320] Support GetPrimaryKeys for Trino Fe
yikf Feb 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 12 additions & 3 deletions kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/hive/Utils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
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;
Expand Down Expand Up @@ -193,12 +194,20 @@ public static JdbcConnectionParams extractURLComponents(String uri, Properties i
}
}

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));
}
}
}

Expand Down Expand Up @@ -477,4 +486,4 @@ public static String getCanonicalHostName(String hostName) {
public static boolean isKyuubiOperationHint(String hint) {
return KYUUBI_OPERATION_HINT_PATTERN.matcher(hint).matches();
}
}
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这段代码主要是对Hive JDBC连接字符串进行解析，提取其中的相关信息并填充到对象中。以下是一些注释：

  • 第 2 行：导入了一些包。
  • 第 9-10 行：引入了 Apache Commons Lang3 库，用于在检查字符串是否为空时方便使用 StringUtils 工具类。
  • 第 12 行：导入了 Thrift 相关的类。
  • 第 13 行：导入了日志相关的类。
  • 第 16-18 行：该方法通过分析 JDBC URL（Uniform Resource Locator）并返回一个 JdbcConnectionParams 对象实例。
  • 第 20-24 行：基于正则表达式来分析 URI（Uniform Resource Identifier）中的每个部分，并添加到 HashMap 对象中以创建一个包含所有属性的连接参数对象。
  • 第 26-32 行：声明并初始化了一个 confPattern 正则表达式用于解析 Hive 配置设置。
  • 第 34-38 行：从 JDBC URI 字符串中提取 Hive 配置设置。
  • 第 40-46 行：使用新的 confPattern 正则表达式替换之前的 pattern 正则表达式。将解析后的参数添加到连接参数对象的映射中。
  • 第 48-57 行：处理连接参数字符串，并将其中的键值对添加到连接参数对象中的 Hive 配置映射中。
  • 第 59-75 行：定义了一些帮助程序函数和正则表达式。

根据这段代码，以下是我的注释：

  • 在第 9-10 行中，StringUtils.isNotBlank 和 StringUtils.contains 都是非常实用的方法。可以使用这些方法来检查字符串是否为空或包含特定字符。
  • 第 26 行中 confPattern 的初始化部分似乎不太对。最后一项是 ";?" 而不是 ";?" 小数点是多余的。
  • 建议将变量命名更改为更具可读性和可维护性。
  • 传入的参数 jdbcURI、info 和 logger 可能存在空引用异常（NullPointerException），建议在使用它们之前进行 null 检查。
  • 程序中可能会出现其他的空引用异常，应逐个检查所有的 HashMap 对象，并确保它们包含了必要的键值映射。

77 changes: 69 additions & 8 deletions kyuubi-hive-jdbc/src/test/java/org/apache/kyuubi/jdbc/hive/UtilsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,13 @@
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;
Expand All @@ -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"
}
});
}
Expand All @@ -61,11 +119,13 @@ public 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;
}

Expand All @@ -76,5 +136,6 @@ public void testExtractURLComponents() throws JdbcUriParseException {
assertEquals(Integer.parseInt(expectedPort), jdbcConnectionParams1.getPort());
assertEquals(expectedCatalog, jdbcConnectionParams1.getCatalogName());
assertEquals(expectedDb, jdbcConnectionParams1.getDbName());
assertEquals(expectedHiveConf, jdbcConnectionParams1.getHiveConfs());
}
}
}