From b27c140944f5c277ac3805f9b132e860f5b81e60 Mon Sep 17 00:00:00 2001 From: yanghua Date: Fri, 14 Apr 2023 20:13:09 +0800 Subject: [PATCH 1/2] Fix the wrong parsing of jvm parameters in jdbc URL --- .../org/apache/kyuubi/jdbc/hive/Utils.java | 15 +++- .../apache/kyuubi/jdbc/hive/UtilsTest.java | 77 +++++++++++++++++-- 2 files changed, 81 insertions(+), 11 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 c5b197f13df..c667d95d7fb 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.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; @@ -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)); + } } } @@ -477,4 +486,4 @@ public static String getCanonicalHostName(String hostName) { public static boolean isKyuubiOperationHint(String hint) { return KYUUBI_OPERATION_HINT_PATTERN.matcher(hint).matches(); } -} +} \ No newline at end of file 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 c890c873190..b0b623249a3 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 @@ 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 expectedHiveConf; private String uri; @Parameterized.Parameters - public static Collection data() { + public static Collection 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().put("k2", "v2").build(), + "jdbc:hive2:///db;k1=v1?k2=v2#k3=v3" + }, + { + "localhost", + "10009", + null, + "default", + new ImmutableMap.Builder().build(), + "jdbc:hive2:///" + }, + { + "localhost", + "10009", + null, + "default", + new ImmutableMap.Builder().build(), + "jdbc:kyuubi://" + }, + { + "localhost", + "10009", + null, + "default", + new ImmutableMap.Builder().build(), + "jdbc:hive2://" + }, + { + "hostname", + "10018", + null, + "db", + new ImmutableMap.Builder().put("k2", "v2").build(), + "jdbc:hive2://hostname:10018/db;k1=v1?k2=v2#k3=v3" + }, { "hostname", "10018", "catalog", "db", + new ImmutableMap.Builder().put("k2", "v2").build(), "jdbc:hive2://hostname:10018/catalog/db;k1=v1?k2=v2#k3=v3" + }, + { + "hostname", + "10018", + "catalog", + "db", + new ImmutableMap.Builder() + .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 UtilsTest( String expectedPort, String expectedCatalog, String expectedDb, + Map 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 void testExtractURLComponents() throws JdbcUriParseException { assertEquals(Integer.parseInt(expectedPort), jdbcConnectionParams1.getPort()); assertEquals(expectedCatalog, jdbcConnectionParams1.getCatalogName()); assertEquals(expectedDb, jdbcConnectionParams1.getDbName()); + assertEquals(expectedHiveConf, jdbcConnectionParams1.getHiveConfs()); } -} +} \ No newline at end of file From 26aa92a3f60831a06d7c1a94fee39432fff5053b Mon Sep 17 00:00:00 2001 From: Yikf Date: Tue, 14 Feb 2023 13:34:55 +0800 Subject: [PATCH 2/2] [KYUUBI #4320] Support GetPrimaryKeys for Trino Fe ### _Why are the changes needed?_ Support GetPrimaryKeys for Trino Fe, close https://github.com/apache/kyuubi/issues/4320 ### _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 #4321 from Yikf/primaryKeys. Closes #4320 3690a2c7 [Yikf] Support GetPrimaryKeys for Trino Fe Authored-by: Yikf Signed-off-by: ulyssesyou --- .../apache/kyuubi/sql/KyuubiTrinoFeBaseLexer.g4 | 6 ++++++ .../kyuubi/sql/KyuubiTrinoFeBaseParser.g4 | 7 +++++++ .../api/KyuubiTrinoOperationTranslator.scala | 7 ++++++- .../parser/trino/KyuubiTrinoFeAstBuilder.scala | 6 +++++- .../sql/plan/trino/TrinoFeOperations.scala | 4 ++++ .../parser/trino/KyuubiTrinoFeParserSuite.scala | 17 ++++++++++++++++- 6 files changed, 44 insertions(+), 3 deletions(-) diff --git a/kyuubi-server/src/main/antlr4/org/apache/kyuubi/sql/KyuubiTrinoFeBaseLexer.g4 b/kyuubi-server/src/main/antlr4/org/apache/kyuubi/sql/KyuubiTrinoFeBaseLexer.g4 index 0b9543a430c..0cc02de6435 100644 --- a/kyuubi-server/src/main/antlr4/org/apache/kyuubi/sql/KyuubiTrinoFeBaseLexer.g4 +++ b/kyuubi-server/src/main/antlr4/org/apache/kyuubi/sql/KyuubiTrinoFeBaseLexer.g4 @@ -97,6 +97,12 @@ SCOPE_TABLE: 'SCOPE_TABLE'; SOURCE_DATA_TYPE: 'SOURCE_DATA_TYPE'; IS_AUTOINCREMENT: 'IS_AUTOINCREMENT'; IS_GENERATEDCOLUMN: 'IS_GENERATEDCOLUMN'; +VARCHAR: 'VARCHAR'; +SMALLINT: 'SMALLINT'; +CAST: 'CAST'; +AS: 'AS'; +KEY_SEQ: 'KEY_SEQ'; +PK_NAME: 'PK_NAME'; fragment SEARCH_STRING_ESCAPE: '\'' '\\' '\''; diff --git a/kyuubi-server/src/main/antlr4/org/apache/kyuubi/sql/KyuubiTrinoFeBaseParser.g4 b/kyuubi-server/src/main/antlr4/org/apache/kyuubi/sql/KyuubiTrinoFeBaseParser.g4 index 590c4378d52..6af00af5de5 100644 --- a/kyuubi-server/src/main/antlr4/org/apache/kyuubi/sql/KyuubiTrinoFeBaseParser.g4 +++ b/kyuubi-server/src/main/antlr4/org/apache/kyuubi/sql/KyuubiTrinoFeBaseParser.g4 @@ -47,6 +47,13 @@ statement SOURCE_DATA_TYPE COMMA IS_AUTOINCREMENT COMMA IS_GENERATEDCOLUMN FROM SYSTEM_JDBC_COLUMNS (WHERE tableCatalogFilter? AND? tableSchemaFilter? AND? tableNameFilter? AND? colNameFilter?)? ORDER BY TABLE_CAT COMMA TABLE_SCHEM COMMA TABLE_NAME COMMA ORDINAL_POSITION #getColumns + | SELECT CAST LEFT_PAREN NULL AS VARCHAR RIGHT_PAREN TABLE_CAT COMMA + CAST LEFT_PAREN NULL AS VARCHAR RIGHT_PAREN TABLE_SCHEM COMMA + CAST LEFT_PAREN NULL AS VARCHAR RIGHT_PAREN TABLE_NAME COMMA + CAST LEFT_PAREN NULL AS VARCHAR RIGHT_PAREN COLUMN_NAME COMMA + CAST LEFT_PAREN NULL AS SMALLINT RIGHT_PAREN KEY_SEQ COMMA + CAST LEFT_PAREN NULL AS VARCHAR RIGHT_PAREN PK_NAME + WHERE FALSE #getPrimaryKeys | .*? #passThrough ; diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/trino/api/KyuubiTrinoOperationTranslator.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/trino/api/KyuubiTrinoOperationTranslator.scala index 6ec9fc1c80e..84686f6b035 100644 --- a/kyuubi-server/src/main/scala/org/apache/kyuubi/server/trino/api/KyuubiTrinoOperationTranslator.scala +++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/server/trino/api/KyuubiTrinoOperationTranslator.scala @@ -25,7 +25,7 @@ import org.apache.kyuubi.operation.OperationHandle import org.apache.kyuubi.service.BackendService import org.apache.kyuubi.sql.parser.trino.KyuubiTrinoFeParser import org.apache.kyuubi.sql.plan.PassThroughNode -import org.apache.kyuubi.sql.plan.trino.{GetCatalogs, GetColumns, GetSchemas, GetTables, GetTableTypes, GetTypeInfo} +import org.apache.kyuubi.sql.plan.trino.{GetCatalogs, GetColumns, GetPrimaryKeys, GetSchemas, GetTables, GetTableTypes, GetTypeInfo} class KyuubiTrinoOperationTranslator(backendService: BackendService) { lazy val parser = new KyuubiTrinoFeParser() @@ -68,6 +68,11 @@ class KyuubiTrinoOperationTranslator(backendService: BackendService) { schemaPattern, tableNamePattern, colNamePattern) + case GetPrimaryKeys() => + val operationHandle = backendService.getPrimaryKeys(sessionHandle, null, null, null) + // The trino implementation always returns empty. + operationHandle.setHasResultSet(false) + operationHandle case PassThroughNode() => backendService.executeStatement(sessionHandle, statement, configs, runAsync, queryTimeout) } diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/sql/parser/trino/KyuubiTrinoFeAstBuilder.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/sql/parser/trino/KyuubiTrinoFeAstBuilder.scala index c5ae9719947..061985c1caf 100644 --- a/kyuubi-server/src/main/scala/org/apache/kyuubi/sql/parser/trino/KyuubiTrinoFeAstBuilder.scala +++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/sql/parser/trino/KyuubiTrinoFeAstBuilder.scala @@ -25,7 +25,7 @@ import org.apache.kyuubi.sql.KyuubiTrinoFeBaseParser._ import org.apache.kyuubi.sql.KyuubiTrinoFeBaseParserBaseVisitor import org.apache.kyuubi.sql.parser.KyuubiParser.unescapeSQLString import org.apache.kyuubi.sql.plan.{KyuubiTreeNode, PassThroughNode} -import org.apache.kyuubi.sql.plan.trino.{GetCatalogs, GetColumns, GetSchemas, GetTables, GetTableTypes, GetTypeInfo} +import org.apache.kyuubi.sql.plan.trino.{GetCatalogs, GetColumns, GetPrimaryKeys, GetSchemas, GetTables, GetTableTypes, GetTypeInfo} class KyuubiTrinoFeAstBuilder extends KyuubiTrinoFeBaseParserBaseVisitor[AnyRef] { @@ -92,6 +92,10 @@ class KyuubiTrinoFeAstBuilder extends KyuubiTrinoFeBaseParserBaseVisitor[AnyRef] GetColumns(catalog, schemaPattern, tableNamePattern, colNamePattern) } + override def visitGetPrimaryKeys(ctx: GetPrimaryKeysContext): KyuubiTreeNode = { + GetPrimaryKeys() + } + override def visitNullCatalog(ctx: NullCatalogContext): AnyRef = { null } diff --git a/kyuubi-server/src/main/scala/org/apache/kyuubi/sql/plan/trino/TrinoFeOperations.scala b/kyuubi-server/src/main/scala/org/apache/kyuubi/sql/plan/trino/TrinoFeOperations.scala index 85e6f168bcb..6136995ab10 100644 --- a/kyuubi-server/src/main/scala/org/apache/kyuubi/sql/plan/trino/TrinoFeOperations.scala +++ b/kyuubi-server/src/main/scala/org/apache/kyuubi/sql/plan/trino/TrinoFeOperations.scala @@ -55,3 +55,7 @@ case class GetColumns( colNamePattern: String) extends KyuubiTreeNode { override def name(): String = "Get Columns" } + +case class GetPrimaryKeys() extends KyuubiTreeNode { + override def name(): String = "Get Primary Keys" +} diff --git a/kyuubi-server/src/test/scala/org/apache/kyuubi/parser/trino/KyuubiTrinoFeParserSuite.scala b/kyuubi-server/src/test/scala/org/apache/kyuubi/parser/trino/KyuubiTrinoFeParserSuite.scala index 3f5cf70b559..bbced0b61ad 100644 --- a/kyuubi-server/src/test/scala/org/apache/kyuubi/parser/trino/KyuubiTrinoFeParserSuite.scala +++ b/kyuubi-server/src/test/scala/org/apache/kyuubi/parser/trino/KyuubiTrinoFeParserSuite.scala @@ -20,7 +20,7 @@ package org.apache.kyuubi.parser.trino import org.apache.kyuubi.KyuubiFunSuite import org.apache.kyuubi.sql.parser.trino.KyuubiTrinoFeParser import org.apache.kyuubi.sql.plan.{KyuubiTreeNode, PassThroughNode} -import org.apache.kyuubi.sql.plan.trino.{GetCatalogs, GetColumns, GetSchemas, GetTables, GetTableTypes, GetTypeInfo} +import org.apache.kyuubi.sql.plan.trino.{GetCatalogs, GetColumns, GetPrimaryKeys, GetSchemas, GetTables, GetTableTypes, GetTypeInfo} class KyuubiTrinoFeParserSuite extends KyuubiFunSuite { val parser = new KyuubiTrinoFeParser() @@ -354,4 +354,19 @@ class KyuubiTrinoFeParserSuite extends KyuubiFunSuite { tableName = "%aa", colName = "%bb") } + + test("Support GetPrimaryKeys for Trino Fe") { + val kyuubiTreeNode = parse( + """ + | SELECT CAST(NULL AS varchar) TABLE_CAT, + | CAST(NULL AS varchar) TABLE_SCHEM, + | CAST(NULL AS varchar) TABLE_NAME, + | CAST(NULL AS varchar) COLUMN_NAME, + | CAST(NULL AS smallint) KEY_SEQ, + | CAST(NULL AS varchar) PK_NAME + | WHERE false + |""".stripMargin) + + assert(kyuubiTreeNode.isInstanceOf[GetPrimaryKeys]) + } }