From 997d72a1cda94e8ea9a43ac5fe341dcfa75ffd67 Mon Sep 17 00:00:00 2001
From: Cheng Pan <chengpan@apache.org>
Date: Fri, 24 Sep 2021 10:25:13 +0800
Subject: [PATCH] [KYUUBI #884][FOLLOWUP] Fix catalog in
 KyuubiDatabaseMetaData#getTables

<!--
Thanks for sending a pull request!

Here are some tips for you:
  1. If this is your first time, please read our contributor guidelines: https://kyuubi.readthedocs.io/en/latest/community/contributions.html
  2. If the PR is related to an issue in https://github.com/apache/incubator-kyuubi/issues, add '[KYUUBI #XXXX]' in your PR title, e.g., '[KYUUBI #XXXX] Your PR title ...'.
  3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][KYUUBI #XXXX] Your PR title ...'.
-->

### _Why are the changes needed?_
<!--
Please clarify why the changes are needed. For instance,
  1. If you add a feature, you can talk about the use case of it.
  2. If you fix a bug, you can clarify why it is a bug.
-->
The behavior of `KyuubiDatabaseMetaData#getTables` changed by accident in #884[1], this PR restores the method's previous behavior and adds multi-catalog test case to verify it works as expected.

[1] [This](https://github.com/apache/incubator-kyuubi/commit/1a8b4ebaaeb8308018cab0c65497c74bc38f4a85#diff-c540737d1b293b909d07d50a05f67d35d274bc3163d723409135634a575e5e9dR68) makes KyuubiDatabaseMetaData#getTables method make no sense, just invokes the superclass.

### _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/latest/develop_tools/testing.html#running-tests) locally before make a pull request

Closes #1133 from pan3793/jdbc.

Closes #884

f02327d9 [Cheng Pan] add provided scala-library
3702a08a [Cheng Pan] nit
18045a92 [Cheng Pan] Fix test jars scope
edd1ca8f [Cheng Pan] Address comments
3fecbf5b [Cheng Pan] Clean withKyuubiConf instead of extraConfigs
45e7bd19 [Cheng Pan] nit
f19def37 [Cheng Pan] [KYUUBI #884][FOLLOWUP] Fix catalog in KyuubiDatabaseMetaData#getTables

Authored-by: Cheng Pan <chengpan@apache.org>
Signed-off-by: Cheng Pan <chengpan@apache.org>
---
 kyuubi-hive-jdbc/pom.xml                      | 14 ++++++
 .../kyuubi/jdbc/KyuubiDatabaseMetaData.java   |  3 +-
 .../kyuubi/jdbc/KyuubiDriverSuite.scala       | 50 +++++++++++++------
 3 files changed, 51 insertions(+), 16 deletions(-)

diff --git a/kyuubi-hive-jdbc/pom.xml b/kyuubi-hive-jdbc/pom.xml
index ec057e2cf..27712a0d4 100644
--- a/kyuubi-hive-jdbc/pom.xml
+++ b/kyuubi-hive-jdbc/pom.xml
@@ -34,6 +34,11 @@
     <packaging>jar</packaging>
 
     <dependencies>
+        <dependency>
+            <groupId>org.scala-lang</groupId>
+            <artifactId>scala-library</artifactId>
+            <scope>provided</scope>
+        </dependency>
 
         <dependency>
             <groupId>org.apache.hive</groupId>
@@ -70,6 +75,7 @@
             <artifactId>kyuubi-common_${scala.binary.version}</artifactId>
             <version>${project.version}</version>
             <type>test-jar</type>
+            <scope>test</scope>
         </dependency>
 
         <dependency>
@@ -78,11 +84,13 @@
             <version>${project.version}</version>
             <scope>test</scope>
         </dependency>
+
         <dependency>
             <groupId>org.apache.kyuubi</groupId>
             <artifactId>kyuubi-spark-sql-engine_${scala.binary.version}</artifactId>
             <version>${project.version}</version>
             <type>test-jar</type>
+            <scope>test</scope>
         </dependency>
 
         <dependency>
@@ -91,6 +99,12 @@
             <scope>test</scope>
         </dependency>
 
+        <dependency>
+            <groupId>org.apache.iceberg</groupId>
+            <artifactId>${iceberg.name}</artifactId>
+            <scope>test</scope>
+        </dependency>
+
         <!--
           Spark requires `commons-collections` and `commons-io` but got them from transitive
           dependencies of `hadoop-client`. As we are using Hadoop Shaded Client, we need add
diff --git a/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/KyuubiDatabaseMetaData.java b/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/KyuubiDatabaseMetaData.java
index dc4aeff49..1a45bf936 100644
--- a/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/KyuubiDatabaseMetaData.java
+++ b/kyuubi-hive-jdbc/src/main/java/org/apache/kyuubi/jdbc/KyuubiDatabaseMetaData.java
@@ -60,11 +60,10 @@ public class KyuubiDatabaseMetaData extends HiveDatabaseMetaData {
         if (tStatus.getStatusCode() != TStatusCode.SUCCESS_STATUS) {
             throw new HiveSQLException(tStatus);
         }
-        new HiveQueryResultSet.Builder(conn)
+        return new HiveQueryResultSet.Builder(conn)
                 .setClient(client)
                 .setSessionHandle(sessHandle)
                 .setStmtHandle(getTableResp.getOperationHandle())
                 .build();
-        return super.getTables(catalog, schemaPattern, tableNamePattern, types);
     }
 }
diff --git a/kyuubi-hive-jdbc/src/test/scala/org/apache/kyuubi/jdbc/KyuubiDriverSuite.scala b/kyuubi-hive-jdbc/src/test/scala/org/apache/kyuubi/jdbc/KyuubiDriverSuite.scala
index e92806d3b..b1f68111f 100644
--- a/kyuubi-hive-jdbc/src/test/scala/org/apache/kyuubi/jdbc/KyuubiDriverSuite.scala
+++ b/kyuubi-hive-jdbc/src/test/scala/org/apache/kyuubi/jdbc/KyuubiDriverSuite.scala
@@ -19,33 +19,55 @@ package org.apache.kyuubi.jdbc
 
 import java.util.Properties
 
+import org.apache.kyuubi.IcebergSuiteMixin
 import org.apache.kyuubi.engine.spark.WithSparkSQLEngine
 import org.apache.kyuubi.engine.spark.shim.SparkCatalogShim
+import org.apache.kyuubi.tags.IcebergTest
 
-class KyuubiDriverSuite extends WithSparkSQLEngine {
+@IcebergTest
+class KyuubiDriverSuite extends WithSparkSQLEngine with IcebergSuiteMixin {
+
+  override def withKyuubiConf: Map[String, String] = extraConfigs -
+    "spark.sql.defaultCatalog" -
+    "spark.sql.catalog.spark_catalog"
+
+  override def afterAll(): Unit = {
+    super.afterAll()
+    for ((k, _) <- withKyuubiConf) {
+      System.clearProperty(k)
+    }
+  }
 
   test("get tables with kyuubi driver") {
     val kyuubiDriver = new KyuubiDriver()
     val connection = kyuubiDriver.connect(getJdbcUrl, new Properties())
+    assert(connection.isInstanceOf[KyuubiConnection])
+    val metaData = connection.getMetaData
+    assert(metaData.isInstanceOf[KyuubiDatabaseMetaData])
     val statement = connection.createStatement()
-    val table = s"${SparkCatalogShim.SESSION_CATALOG}.default.kyuubi_hive_jdbc"
+    val table1 = s"${SparkCatalogShim.SESSION_CATALOG}.default.kyuubi_hive_jdbc"
+    val table2 = s"$catalog.default.hdp_cat_tbl"
     try {
-      statement.execute(s"CREATE TABLE ${table}(key int) using parquet")
-      assert(connection.isInstanceOf[KyuubiConnection])
-      val metaData = connection.getMetaData
-      assert(metaData.isInstanceOf[KyuubiDatabaseMetaData])
-      val resultSet = metaData.getTables(SparkCatalogShim.SESSION_CATALOG, "default", "%", null)
-      assert(resultSet.next())
-      assert(resultSet.getString(1) === SparkCatalogShim.SESSION_CATALOG)
-      assert(resultSet.getString(2) === "default")
-      assert(resultSet.getString(3) === "kyuubi_hive_jdbc")
+      statement.execute(s"CREATE TABLE $table1(key int) USING parquet")
+      statement.execute(s"CREATE TABLE $table2(key int) USING $format")
+
+      val resultSet1 = metaData.getTables(SparkCatalogShim.SESSION_CATALOG, "default", "%", null)
+      assert(resultSet1.next())
+      assert(resultSet1.getString(1) === SparkCatalogShim.SESSION_CATALOG)
+      assert(resultSet1.getString(2) === "default")
+      assert(resultSet1.getString(3) === "kyuubi_hive_jdbc")
+
+      val resultSet2 = metaData.getTables(catalog, "default", "%", null)
+      assert(resultSet2.next())
+      assert(resultSet2.getString(1) === catalog)
+      assert(resultSet2.getString(2) === "default")
+      assert(resultSet2.getString(3) === "hdp_cat_tbl")
     } finally {
-      statement.execute(s"DROP TABLE ${table}")
+      statement.execute(s"DROP TABLE IF EXISTS $table1")
+      statement.execute(s"DROP TABLE IF EXISTS $table2")
       statement.close()
       connection.close()
     }
     connection
   }
-
-  override def withKyuubiConf: Map[String, String] = Map.empty
 }