From a4d507a9c61b4bb2c4d94880fd7bb6c158e35376 Mon Sep 17 00:00:00 2001 From: Kent Yao Date: Tue, 2 Feb 2021 09:54:47 +0800 Subject: [PATCH] [KYUUBI #337] [FOLLOWUP]GetSchemas supports DSv2 multipart namespaces with dot backtick MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ![yaooqinn](https://badgen.net/badge/Hello/yaooqinn/green) [![Closes #337](https://badgen.net/badge/Preview/Closes%20%23337/blue)](https://github.com/yaooqinn/kyuubi/pull/337) ![23](https://badgen.net/badge/%2B/23/red) ![5](https://badgen.net/badge/-/5/green) ![3](https://badgen.net/badge/commits/3/yellow) ![Target Issue](https://badgen.net/badge/Missing/Target%20Issue/ff0000) ![Bug](https://badgen.net/badge/Label/Bug/) [❨?❩](https://pullrequestbadge.com/?utm_medium=github&utm_source=yaooqinn&utm_campaign=badge_info) ### _Why are the changes needed?_ add support for '.' and '`' in a namespace part for instance, cases like https://github.com/yaooqinn/kyuubi/pull/337/files#diff-bcd0dfb958c3943a58a9705ac7053374c796449fefd6be7e9f014f9dd14b558fR94 produce the below catalog tree, we should treat `a.b`.c and a.`b.c` differently. ``` /var/folders/01/h81cs4sn3dq2dd_k4j6fhrmc0000gn/T/kyuubi-94c946f7-a64b-4980-9dd1-a63a573244dd ├── a │   └── b.c ├── a.b │   └── c ├── a.b.c ├── a.b`.c ├── db1 │   └── db2 │   └── db3 └── db4 ``` ### _How was this patch tested?_ - [x] Add some test cases that check the changes thoroughly including negative and positive cases if possible - [x] Add screenshots for manual tests if appropriate - [x] [Run test](https://kyuubi.readthedocs.io/en/latest/tools/testing.html#running-tests) locally before make a pull request Closes #337 from yaooqinn/getschema3. fdb8b19 [Kent Yao] [FOLLOWUP]GetSchemas supports DSv2 multipart namespaces 0091cfc [Kent Yao] [FOLLOWUP]GetSchemas supports DSv2 multipart namespaces 5ccee06 [Kent Yao] [FOLLOWUP]GetSchemas supports DSv2 multipart namespaces Authored-by: Kent Yao Signed-off-by: Kent Yao --- .../kyuubi/engine/spark/shim/Shim_v3_0.scala | 15 +++++++++++++-- .../kyuubi/operation/BasicIcebergJDBCTests.scala | 13 ++++++++++--- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/shim/Shim_v3_0.scala b/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/shim/Shim_v3_0.scala index 9dc75a6dd..a99e2b968 100644 --- a/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/shim/Shim_v3_0.scala +++ b/externals/kyuubi-spark-sql-engine/src/main/scala/org/apache/kyuubi/engine/spark/shim/Shim_v3_0.scala @@ -62,9 +62,20 @@ class Shim_v3_0 extends Shim_v2_4 { catalog.listNamespaces(ns) } if (children.isEmpty) { - namespaces + namespaces.map(_.map(quoteIfNeeded)) } else { - namespaces ++: listNamespaces(catalog, children) + namespaces.map(_.map(quoteIfNeeded)) ++: listNamespaces(catalog, children) + } + } + + /** + * Forked from Apache Spark's org.apache.spark.sql.connector.catalog.CatalogV2Implicits + */ + private def quoteIfNeeded(part: String): String = { + if (part.contains(".") || part.contains("`")) { + s"`${part.replace("`", "``")}`" + } else { + part } } diff --git a/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/BasicIcebergJDBCTests.scala b/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/BasicIcebergJDBCTests.scala index 87967a57c..a1e5210b4 100644 --- a/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/BasicIcebergJDBCTests.scala +++ b/kyuubi-common/src/test/scala/org/apache/kyuubi/operation/BasicIcebergJDBCTests.scala @@ -91,7 +91,14 @@ trait BasicIcebergJDBCTests extends JDBCTestUtils { } test("get schemas with multipart namespaces") { - val dbs = Seq("db1", "db1.db2", "db1.db2.db3", "db4") + val dbs = Seq( + "`a.b`", + "`a.b`.c", + "a.`b.c`", + "`a.b.c`", + "`a.b``.c`", + "db1.db2.db3", + "db4") withDatabases(dbs: _*) { statement => dbs.foreach(db => statement.execute(s"CREATE NAMESPACE IF NOT EXISTS $db")) @@ -100,8 +107,8 @@ trait BasicIcebergJDBCTests extends JDBCTestUtils { val allPattern = Seq("", "*", "%", null, ".*", "_*", "_%", ".%") Seq(null, catalog).foreach { cg => allPattern foreach { pattern => - checkGetSchemas( - metaData.getSchemas(cg, pattern), dbs ++ Seq("global_temp"), catalog) + checkGetSchemas(metaData.getSchemas(cg, pattern), + dbs ++ Seq("global_temp", "a", "db1", "db1.db2"), catalog) } }