You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "pan3793 (via GitHub)" <gi...@apache.org> on 2025/11/11 07:25:54 UTC

[PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

pan3793 opened a new pull request, #52995:
URL: https://github.com/apache/spark/pull/52995

   <!--
   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://spark.apache.org/contributing.html
     2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
     3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
     4. Be sure to keep the PR description updated to reflect all changes.
     5. Please write your PR title to summarize what this PR proposes.
     6. If possible, provide a concise example to reproduce the issue for a faster review.
     7. If you want to add a new configuration, please read the guideline first for naming configurations in
        'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
     8. If you want to add or modify an error type or message, please read the guideline first in
        'common/utils/src/main/resources/error/README.md'.
   -->
   
   ### What changes were proposed in this pull request?
   <!--
   Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue. 
   If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
     1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
     2. If you fix some SQL features, you can provide some references of other DBMSes.
     3. If there is design documentation, please add the link.
     4. If there is a discussion in the mailing list, please add the link.
   -->
   Implement `getTableTypes` and `getTables` methods defined in `java.sql.DatabaseMetaData` for `SparkConnectDatabaseMetaData`.
   
   ```java
       /**
        * Retrieves the table types available in this database.  The results
        * are ordered by table type.
        *
        * <P>The table type is:
        *  <OL>
        *  <LI><B>TABLE_TYPE</B> String {@code =>} table type.  Typical types are "TABLE",
        *                  "VIEW", "SYSTEM TABLE", "GLOBAL TEMPORARY",
        *                  "LOCAL TEMPORARY", "ALIAS", "SYNONYM".
        *  </OL>
        *
        * @return a {@code ResultSet} object in which each row has a
        *         single {@code String} column that is a table type
        * @throws SQLException if a database access error occurs
        */
       ResultSet getTableTypes() throws SQLException;
   
       /**
        * Retrieves a description of the tables available in the given catalog.
        * Only table descriptions matching the catalog, schema, table
        * name and type criteria are returned.  They are ordered by
        * {@code TABLE_TYPE}, {@code TABLE_CAT},
        * {@code TABLE_SCHEM} and {@code TABLE_NAME}.
        * <P>
        * Each table description has the following columns:
        *  <OL>
        *  <LI><B>TABLE_CAT</B> String {@code =>} table catalog (may be {@code null})
        *  <LI><B>TABLE_SCHEM</B> String {@code =>} table schema (may be {@code null})
        *  <LI><B>TABLE_NAME</B> String {@code =>} table name
        *  <LI><B>TABLE_TYPE</B> String {@code =>} table type.  Typical types are "TABLE",
        *                  "VIEW", "SYSTEM TABLE", "GLOBAL TEMPORARY",
        *                  "LOCAL TEMPORARY", "ALIAS", "SYNONYM".
        *  <LI><B>REMARKS</B> String {@code =>} explanatory comment on the table (may be {@code null})
        *  <LI><B>TYPE_CAT</B> String {@code =>} the types catalog (may be {@code null})
        *  <LI><B>TYPE_SCHEM</B> String {@code =>} the types schema (may be {@code null})
        *  <LI><B>TYPE_NAME</B> String {@code =>} type name (may be {@code null})
        *  <LI><B>SELF_REFERENCING_COL_NAME</B> String {@code =>} name of the designated
        *                  "identifier" column of a typed table (may be {@code null})
        *  <LI><B>REF_GENERATION</B> String {@code =>} specifies how values in
        *                  SELF_REFERENCING_COL_NAME are created. Values are
        *                  "SYSTEM", "USER", "DERIVED". (may be {@code null})
        *  </OL>
        *
        * <P><B>Note:</B> Some databases may not return information for
        * all tables.
        *
        * @param catalog a catalog name; must match the catalog name as it
        *        is stored in the database; "" retrieves those without a catalog;
        *        {@code null} means that the catalog name should not be used to narrow
        *        the search
        * @param schemaPattern a schema name pattern; must match the schema name
        *        as it is stored in the database; "" retrieves those without a schema;
        *        {@code null} means that the schema name should not be used to narrow
        *        the search
        * @param tableNamePattern a table name pattern; must match the
        *        table name as it is stored in the database
        * @param types a list of table types, which must be from the list of table types
        *         returned from {@link #getTableTypes},to include; {@code null} returns
        * all types
        * @return {@code ResultSet} - each row is a table description
        * @throws SQLException if a database access error occurs
        * @see #getSearchStringEscape
        */
       ResultSet getTables(String catalog, String schemaPattern,
                           String tableNamePattern, String types[]) throws SQLException;
   ```
   
   ### Why are the changes needed?
   <!--
   Please clarify why the changes are needed. For instance,
     1. If you propose a new API, clarify the use case for a new API.
     2. If you fix a bug, you can clarify why it is a bug.
   -->
   Enhance API coverage of the Connect JDBC driver, for example, `get[Catalogs|Schemas|Tables|...]` APIs are used by SQL GUI tools such as DBeaver for displaying the tree category.
   
   ### Does this PR introduce _any_ user-facing change?
   <!--
   Note that it means *any* user-facing change including all aspects such as new features, bug fixes, or other behavior changes. Documentation-only updates are not considered user-facing changes.
   
   If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
   If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
   If no, write 'No'.
   -->
   No, Connect JDBC driver is a new feature under development.
   
   ### How was this patch tested?
   <!--
   If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
   If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
   If tests were not added, please describe why they were not added and/or why it was difficult to add.
   If benchmark tests were added, please run the benchmarks in GitHub Actions for the consistent environment, and the instructions could accord to: https://spark.apache.org/developer-tools.html#github-workflow-benchmarks.
   -->
   TODO Add UT.
   
   Tested via DBeaver - the catalog/schema/table tree works now.
   
   <img width="1260" height="892" alt="Xnip2025-11-11_15-21-10" src="https://github.com/user-attachments/assets/4e8f7f52-ba7d-41ff-8adc-4b29c309e04e" />
   
   ### Was this patch authored or co-authored using generative AI tooling?
   <!--
   If generative AI tooling has been used in the process of authoring this patch, please include the
   phrase: 'Generated-by: ' followed by the name of the tool and its version.
   If no, write 'No'.
   Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details.
   -->
   No.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #52995:
URL: https://github.com/apache/spark/pull/52995#discussion_r2516549231


##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaData.scala:
##########
@@ -618,4 +706,6 @@ object SparkConnectDatabaseMetaData {
     "XMLFOREST", "XMLNAMESPACES", "XMLPARSE", "XMLPI", "XMLROOT", "XMLSERIALIZE",
     "YEAR"
   )
+
+  private[jdbc] val TABLE_TYPES = Seq("TABLE", "VIEW")

Review Comment:
   since Spark does not support information schema yet, I have to hard-code it on the client side for now.
   
   cc @cloud-fan @wangyum, do you have better ideas?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #52995:
URL: https://github.com/apache/spark/pull/52995#issuecomment-3520226268

   @dongjoon-hyun, thank you for merging, and I will review all implemented API again during the QA period.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun closed pull request #52995: [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData
URL: https://github.com/apache/spark/pull/52995


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #52995:
URL: https://github.com/apache/spark/pull/52995#discussion_r2516558178


##########
sql/connect/client/jdbc/src/test/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaDataSuite.scala:
##########
@@ -343,4 +343,208 @@ class SparkConnectDatabaseMetaDataSuite extends ConnectFunSuite with RemoteSpark
       }
     }
   }
+
+  test("SparkConnectDatabaseMetaData getTableTypes") {
+    withConnection { conn =>
+      val metadata = conn.getMetaData
+      Using.resource(metadata.getTableTypes) { rs =>
+        val types = new Iterator[String] {
+          def hasNext: Boolean = rs.next()
+          def next(): String = rs.getString("TABLE_TYPE")
+        }.toSeq
+        // results are ordered by TABLE_TYPE
+        assert(types === Seq("TABLE", "VIEW"))
+      }
+    }
+  }
+
+  test("SparkConnectDatabaseMetaData getTables") {
+
+    case class GetTableResult(
+       TABLE_CAT: String,
+       TABLE_SCHEM: String,
+       TABLE_NAME: String,
+       TABLE_TYPE: String,
+       REMARKS: String,
+       TYPE_CAT: String,
+       TYPE_SCHEM: String,
+       TYPE_NAME: String,
+       SELF_REFERENCING_COL_NAME: String,
+       REF_GENERATION: String)
+
+    def verifyEmptyStringFields(result: GetTableResult): Unit = {
+      assert(result.REMARKS === "")
+      assert(result.TYPE_CAT === "")
+      assert(result.TYPE_SCHEM === "")
+      assert(result.TYPE_NAME === "")
+      assert(result.SELF_REFERENCING_COL_NAME === "")
+      assert(result.REF_GENERATION === "")
+    }
+
+    def verifyGetTables(
+        getTables: () => ResultSet)(verify: Seq[GetTableResult] => Unit): Unit = {
+      Using.resource(getTables()) { rs =>
+        val getTableResults = new Iterator[GetTableResult] {
+          def hasNext: Boolean = rs.next()
+          def next(): GetTableResult = GetTableResult(
+            TABLE_CAT = rs.getString("TABLE_CAT"),
+            TABLE_SCHEM = rs.getString("TABLE_SCHEM"),
+            TABLE_NAME = rs.getString("TABLE_NAME"),
+            TABLE_TYPE = rs.getString("TABLE_TYPE"),
+            REMARKS = rs.getString("REMARKS"),
+            TYPE_CAT = rs.getString("TYPE_CAT"),
+            TYPE_SCHEM = rs.getString("TYPE_SCHEM"),
+            TYPE_NAME = rs.getString("TYPE_NAME"),
+            SELF_REFERENCING_COL_NAME = rs.getString("SELF_REFERENCING_COL_NAME"),
+            REF_GENERATION = rs.getString("REF_GENERATION"))
+        }.toSeq
+        verify(getTableResults)
+      }
+    }
+
+    withConnection { conn =>
+      implicit val spark: SparkSession = conn.asInstanceOf[SparkConnectConnection].spark
+
+      // this catalog does not support view
+      registerCatalog("testcat", TEST_IN_MEMORY_CATALOG)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS testcat.t_db1")
+      spark.sql("CREATE TABLE IF NOT EXISTS testcat.t_db1.t_t1 (id INT)")
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db1")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db1.t1 (id INT)")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db1.t_2 (id INT)")
+      spark.sql(
+        """CREATE VIEW IF NOT EXISTS spark_catalog.db1.t1_v AS
+          |SELECT id FROM spark_catalog.db1.t1
+          |""".stripMargin)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db_2")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db_2.t_2 (id INT)")
+      spark.sql(
+        """CREATE VIEW IF NOT EXISTS spark_catalog.db_2.t_2_v AS
+          |SELECT id FROM spark_catalog.db_2.t_2
+          |""".stripMargin)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db_")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db_.t_ (id INT)")
+
+      val metadata = conn.getMetaData
+
+      // no need to care about "testcat" because it is memory based and session isolated,
+      // also is inaccessible from another SparkSession
+      withDatabase("spark_catalog.db1", "spark_catalog.db_2", "spark_catalog.db_") {
+        // list tables in all catalogs and schemas
+        val getTablesInAllCatalogsAndSchemas = List(null, "%").flatMap { database =>
+          List(null, "%").flatMap { table =>
+            List(null, Array("TABLE", "VIEW")).map { tableTypes =>
+              () => metadata.getTables(null, database, table, tableTypes)
+            }
+          }
+        }
+
+        getTablesInAllCatalogsAndSchemas.foreach { getTables =>
+          verifyGetTables(getTables) { getTableResults =>
+            // results are ordered by TABLE_TYPE, TABLE_CAT, TABLE_SCHEM and TABLE_NAME
+            assert {
+              getTableResults.map { result =>
+                (result.TABLE_TYPE, result.TABLE_CAT, result.TABLE_SCHEM, result.TABLE_NAME)
+              } === Seq(
+                ("TABLE", "spark_catalog", "db1", "t1"),
+                ("TABLE", "spark_catalog", "db1", "t_2"),
+                ("TABLE", "spark_catalog", "db_", "t_"),
+                ("TABLE", "spark_catalog", "db_2", "t_2"),
+                ("TABLE", "testcat", "t_db1", "t_t1"),
+                ("VIEW", "spark_catalog", "db1", "t1_v"),
+                ("VIEW", "spark_catalog", "db_2", "t_2_v"))
+            }
+            getTableResults.foreach(verifyEmptyStringFields)
+          }
+        }
+
+        // list tables with table types
+        val se = intercept[SQLException] {
+          metadata.getTables("spark_catalog", "foo", "bar", Array("TABLE", "METERIALIZED VIEW"))

Review Comment:
   fixed



##########
sql/connect/client/jdbc/src/test/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaDataSuite.scala:
##########
@@ -343,4 +343,208 @@ class SparkConnectDatabaseMetaDataSuite extends ConnectFunSuite with RemoteSpark
       }
     }
   }
+
+  test("SparkConnectDatabaseMetaData getTableTypes") {
+    withConnection { conn =>
+      val metadata = conn.getMetaData
+      Using.resource(metadata.getTableTypes) { rs =>
+        val types = new Iterator[String] {
+          def hasNext: Boolean = rs.next()
+          def next(): String = rs.getString("TABLE_TYPE")
+        }.toSeq
+        // results are ordered by TABLE_TYPE
+        assert(types === Seq("TABLE", "VIEW"))
+      }
+    }
+  }
+
+  test("SparkConnectDatabaseMetaData getTables") {
+
+    case class GetTableResult(
+       TABLE_CAT: String,
+       TABLE_SCHEM: String,
+       TABLE_NAME: String,
+       TABLE_TYPE: String,
+       REMARKS: String,
+       TYPE_CAT: String,
+       TYPE_SCHEM: String,
+       TYPE_NAME: String,
+       SELF_REFERENCING_COL_NAME: String,
+       REF_GENERATION: String)
+
+    def verifyEmptyStringFields(result: GetTableResult): Unit = {
+      assert(result.REMARKS === "")
+      assert(result.TYPE_CAT === "")
+      assert(result.TYPE_SCHEM === "")
+      assert(result.TYPE_NAME === "")
+      assert(result.SELF_REFERENCING_COL_NAME === "")
+      assert(result.REF_GENERATION === "")
+    }
+
+    def verifyGetTables(
+        getTables: () => ResultSet)(verify: Seq[GetTableResult] => Unit): Unit = {
+      Using.resource(getTables()) { rs =>
+        val getTableResults = new Iterator[GetTableResult] {
+          def hasNext: Boolean = rs.next()
+          def next(): GetTableResult = GetTableResult(
+            TABLE_CAT = rs.getString("TABLE_CAT"),
+            TABLE_SCHEM = rs.getString("TABLE_SCHEM"),
+            TABLE_NAME = rs.getString("TABLE_NAME"),
+            TABLE_TYPE = rs.getString("TABLE_TYPE"),
+            REMARKS = rs.getString("REMARKS"),
+            TYPE_CAT = rs.getString("TYPE_CAT"),
+            TYPE_SCHEM = rs.getString("TYPE_SCHEM"),
+            TYPE_NAME = rs.getString("TYPE_NAME"),
+            SELF_REFERENCING_COL_NAME = rs.getString("SELF_REFERENCING_COL_NAME"),
+            REF_GENERATION = rs.getString("REF_GENERATION"))
+        }.toSeq
+        verify(getTableResults)
+      }
+    }
+
+    withConnection { conn =>
+      implicit val spark: SparkSession = conn.asInstanceOf[SparkConnectConnection].spark
+
+      // this catalog does not support view
+      registerCatalog("testcat", TEST_IN_MEMORY_CATALOG)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS testcat.t_db1")
+      spark.sql("CREATE TABLE IF NOT EXISTS testcat.t_db1.t_t1 (id INT)")
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db1")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db1.t1 (id INT)")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db1.t_2 (id INT)")
+      spark.sql(
+        """CREATE VIEW IF NOT EXISTS spark_catalog.db1.t1_v AS
+          |SELECT id FROM spark_catalog.db1.t1
+          |""".stripMargin)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db_2")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db_2.t_2 (id INT)")
+      spark.sql(
+        """CREATE VIEW IF NOT EXISTS spark_catalog.db_2.t_2_v AS
+          |SELECT id FROM spark_catalog.db_2.t_2
+          |""".stripMargin)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db_")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db_.t_ (id INT)")
+
+      val metadata = conn.getMetaData
+
+      // no need to care about "testcat" because it is memory based and session isolated,
+      // also is inaccessible from another SparkSession
+      withDatabase("spark_catalog.db1", "spark_catalog.db_2", "spark_catalog.db_") {
+        // list tables in all catalogs and schemas
+        val getTablesInAllCatalogsAndSchemas = List(null, "%").flatMap { database =>
+          List(null, "%").flatMap { table =>
+            List(null, Array("TABLE", "VIEW")).map { tableTypes =>
+              () => metadata.getTables(null, database, table, tableTypes)
+            }
+          }
+        }
+
+        getTablesInAllCatalogsAndSchemas.foreach { getTables =>
+          verifyGetTables(getTables) { getTableResults =>
+            // results are ordered by TABLE_TYPE, TABLE_CAT, TABLE_SCHEM and TABLE_NAME
+            assert {
+              getTableResults.map { result =>
+                (result.TABLE_TYPE, result.TABLE_CAT, result.TABLE_SCHEM, result.TABLE_NAME)
+              } === Seq(
+                ("TABLE", "spark_catalog", "db1", "t1"),
+                ("TABLE", "spark_catalog", "db1", "t_2"),
+                ("TABLE", "spark_catalog", "db_", "t_"),
+                ("TABLE", "spark_catalog", "db_2", "t_2"),
+                ("TABLE", "testcat", "t_db1", "t_t1"),
+                ("VIEW", "spark_catalog", "db1", "t1_v"),
+                ("VIEW", "spark_catalog", "db_2", "t_2_v"))
+            }
+            getTableResults.foreach(verifyEmptyStringFields)
+          }
+        }
+
+        // list tables with table types
+        val se = intercept[SQLException] {
+          metadata.getTables("spark_catalog", "foo", "bar", Array("TABLE", "METERIALIZED VIEW"))
+        }
+        assert(se.getMessage ===
+          "The requested table types contains unsupported items: METERIALIZED VIEW. " +

Review Comment:
   fixed



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #52995:
URL: https://github.com/apache/spark/pull/52995#discussion_r2517074397


##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaData.scala:
##########
@@ -355,20 +356,124 @@ class SparkConnectDatabaseMetaData(conn: SparkConnectConnection) extends Databas
   override def getSchemas(catalog: String, schemaPattern: String): ResultSet = {
     conn.checkOpen()
 
-    val df = getSchemasDataFrame(catalog, schemaPattern)
+    val df = getSchemasDataFrame(catalog, Some(schemaPattern))
       .orderBy("TABLE_CATALOG", "TABLE_SCHEM")
     new SparkConnectResultSet(df.collectResult())
   }
 
-  override def getTableTypes: ResultSet =
-    throw new SQLFeatureNotSupportedException
+  override def getTableTypes: ResultSet = {
+    conn.checkOpen()
+
+    val df = TABLE_TYPES.toDF("TABLE_TYPE")
+      .orderBy("TABLE_TYPE")
+    new SparkConnectResultSet(df.collectResult())
+  }
+
+  // Schema of the returned DataFrame is:
+  // |-- TABLE_CAT: string (nullable = false)
+  // |-- TABLE_SCHEM: string (nullable = false)
+  // |-- TABLE_NAME: string (nullable = false)
+  // |-- TABLE_TYPE: string (nullable = false)
+  // |-- REMARKS: string (nullable = false)
+  // |-- TYPE_CAT: string (nullable = false)
+  // |-- TYPE_SCHEM: string (nullable = false)
+  // |-- TYPE_NAME: string (nullable = false)
+  // |-- SELF_REFERENCING_COL_NAME: string (nullable = false)
+  // |-- REF_GENERATION: string (nullable = false)
+  private def getTablesDataFrame(
+      catalog: String,
+      schemaPattern: String,
+      tableNamePattern: String): connect.DataFrame = {
+
+    val catalogSchemasDf = if (schemaPattern == "") {

Review Comment:
   `null` is carried by `Some(null)`, this is covered by UT.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #52995:
URL: https://github.com/apache/spark/pull/52995#discussion_r2517037345


##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaData.scala:
##########
@@ -355,20 +356,124 @@ class SparkConnectDatabaseMetaData(conn: SparkConnectConnection) extends Databas
   override def getSchemas(catalog: String, schemaPattern: String): ResultSet = {
     conn.checkOpen()
 
-    val df = getSchemasDataFrame(catalog, schemaPattern)
+    val df = getSchemasDataFrame(catalog, Some(schemaPattern))
       .orderBy("TABLE_CATALOG", "TABLE_SCHEM")
     new SparkConnectResultSet(df.collectResult())
   }
 
-  override def getTableTypes: ResultSet =
-    throw new SQLFeatureNotSupportedException
+  override def getTableTypes: ResultSet = {
+    conn.checkOpen()
+
+    val df = TABLE_TYPES.toDF("TABLE_TYPE")
+      .orderBy("TABLE_TYPE")
+    new SparkConnectResultSet(df.collectResult())
+  }
+
+  // Schema of the returned DataFrame is:
+  // |-- TABLE_CAT: string (nullable = false)
+  // |-- TABLE_SCHEM: string (nullable = false)
+  // |-- TABLE_NAME: string (nullable = false)
+  // |-- TABLE_TYPE: string (nullable = false)
+  // |-- REMARKS: string (nullable = false)
+  // |-- TYPE_CAT: string (nullable = false)
+  // |-- TYPE_SCHEM: string (nullable = false)
+  // |-- TYPE_NAME: string (nullable = false)
+  // |-- SELF_REFERENCING_COL_NAME: string (nullable = false)
+  // |-- REF_GENERATION: string (nullable = false)
+  private def getTablesDataFrame(
+      catalog: String,
+      schemaPattern: String,
+      tableNamePattern: String): connect.DataFrame = {
+
+    val catalogSchemasDf = if (schemaPattern == "") {

Review Comment:
   If `null` is allowed, I think line 391 should be `Option(schemaPattern)`,   otherwise, the `getSchemasDataFrame` function will fail to match any case.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #52995:
URL: https://github.com/apache/spark/pull/52995#discussion_r2516554741


##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaData.scala:
##########
@@ -355,20 +356,124 @@ class SparkConnectDatabaseMetaData(conn: SparkConnectConnection) extends Databas
   override def getSchemas(catalog: String, schemaPattern: String): ResultSet = {
     conn.checkOpen()
 
-    val df = getSchemasDataFrame(catalog, schemaPattern)
+    val df = getSchemasDataFrame(catalog, Some(schemaPattern))
       .orderBy("TABLE_CATALOG", "TABLE_SCHEM")
     new SparkConnectResultSet(df.collectResult())
   }
 
-  override def getTableTypes: ResultSet =
-    throw new SQLFeatureNotSupportedException
+  override def getTableTypes: ResultSet = {
+    conn.checkOpen()
+
+    val df = TABLE_TYPES.toDF("TABLE_TYPE")
+      .orderBy("TABLE_TYPE")
+    new SparkConnectResultSet(df.collectResult())
+  }
+
+  // Schema of the returned DataFrame is:
+  // |-- TABLE_CAT: string (nullable = false)
+  // |-- TABLE_SCHEM: string (nullable = false)
+  // |-- TABLE_NAME: string (nullable = false)
+  // |-- TABLE_TYPE: string (nullable = false)
+  // |-- REMARKS: string (nullable = false)
+  // |-- TYPE_CAT: string (nullable = false)
+  // |-- TYPE_SCHEM: string (nullable = false)
+  // |-- TYPE_NAME: string (nullable = false)
+  // |-- SELF_REFERENCING_COL_NAME: string (nullable = false)
+  // |-- REF_GENERATION: string (nullable = false)
+  private def getTablesDataFrame(
+      catalog: String,
+      schemaPattern: String,
+      tableNamePattern: String): connect.DataFrame = {
+
+    val catalogSchemasDf = if (schemaPattern == "") {

Review Comment:
   `null` is allowed here, see line 323



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #52995:
URL: https://github.com/apache/spark/pull/52995#discussion_r2517037345


##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaData.scala:
##########
@@ -355,20 +356,124 @@ class SparkConnectDatabaseMetaData(conn: SparkConnectConnection) extends Databas
   override def getSchemas(catalog: String, schemaPattern: String): ResultSet = {
     conn.checkOpen()
 
-    val df = getSchemasDataFrame(catalog, schemaPattern)
+    val df = getSchemasDataFrame(catalog, Some(schemaPattern))
       .orderBy("TABLE_CATALOG", "TABLE_SCHEM")
     new SparkConnectResultSet(df.collectResult())
   }
 
-  override def getTableTypes: ResultSet =
-    throw new SQLFeatureNotSupportedException
+  override def getTableTypes: ResultSet = {
+    conn.checkOpen()
+
+    val df = TABLE_TYPES.toDF("TABLE_TYPE")
+      .orderBy("TABLE_TYPE")
+    new SparkConnectResultSet(df.collectResult())
+  }
+
+  // Schema of the returned DataFrame is:
+  // |-- TABLE_CAT: string (nullable = false)
+  // |-- TABLE_SCHEM: string (nullable = false)
+  // |-- TABLE_NAME: string (nullable = false)
+  // |-- TABLE_TYPE: string (nullable = false)
+  // |-- REMARKS: string (nullable = false)
+  // |-- TYPE_CAT: string (nullable = false)
+  // |-- TYPE_SCHEM: string (nullable = false)
+  // |-- TYPE_NAME: string (nullable = false)
+  // |-- SELF_REFERENCING_COL_NAME: string (nullable = false)
+  // |-- REF_GENERATION: string (nullable = false)
+  private def getTablesDataFrame(
+      catalog: String,
+      schemaPattern: String,
+      tableNamePattern: String): connect.DataFrame = {
+
+    val catalogSchemasDf = if (schemaPattern == "") {

Review Comment:
   If `null` is allowed, I think line 391 should be `Option(schemaPattern)`.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #52995:
URL: https://github.com/apache/spark/pull/52995#discussion_r2513168663


##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaData.scala:
##########
@@ -618,4 +706,6 @@ object SparkConnectDatabaseMetaData {
     "XMLFOREST", "XMLNAMESPACES", "XMLPARSE", "XMLPI", "XMLROOT", "XMLSERIALIZE",
     "YEAR"
   )
+
+  private[jdbc] val TABLE_TYPES = Seq("TABLE", "VIEW")

Review Comment:
   this follows Spark Thrift Server behavior (see SPARK-28293 for more details): `o.a.s.sql.hive.thriftserver.SparkGetTableTypesOperation` returns `Array("TABLE", "VIEW")`
   
   as a reference, HiveServer2 has a configuration `hive.server2.table.type.mapping`  to control the returned values
   
   https://github.com/apache/hive/blob/rel/release-2.3.10/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java#L2583-L2587
   
   ```
   HIVE_SERVER2_TABLE_TYPE_MAPPING("hive.server2.table.type.mapping", "CLASSIC", new StringSet("CLASSIC", "HIVE"),
     "This setting reflects how HiveServer2 will report the table types for JDBC and other\n" +
     "client implementations that retrieve the available tables and supported table types\n" +
     "  HIVE : Exposes Hive's native table types like MANAGED_TABLE, EXTERNAL_TABLE, VIRTUAL_VIEW\n" +
     "  CLASSIC : More generic types like TABLE and VIEW"),
   ```
   
   Databricks JDBC driver supports more table types: "SYSTEM TABLE", "TABLE", "VIEW", "METRIC_VIEW"
   
   https://github.com/databricks/databricks-jdbc/blob/v3.0.3/src/main/java/com/databricks/jdbc/common/MetadataResultConstants.java#L249
   
   https://github.com/databricks/databricks-jdbc/blob/v3.0.3/src/main/java/com/databricks/jdbc/common/MetadataResultConstants.java#L13



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on PR #52995:
URL: https://github.com/apache/spark/pull/52995#issuecomment-3519678505

   this is ready for review, cc @LuciferYang @wangyum 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "pan3793 (via GitHub)" <gi...@apache.org>.
pan3793 commented on code in PR #52995:
URL: https://github.com/apache/spark/pull/52995#discussion_r2513938399


##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaData.scala:
##########
@@ -360,15 +361,113 @@ class SparkConnectDatabaseMetaData(conn: SparkConnectConnection) extends Databas
     new SparkConnectResultSet(df.collectResult())
   }
 
-  override def getTableTypes: ResultSet =
-    throw new SQLFeatureNotSupportedException
+  override def getTableTypes: ResultSet = {
+    conn.checkOpen()
+
+    val df = TABLE_TYPES.toDF("TABLE_TYPE")
+      .orderBy("TABLE_TYPE")
+    new SparkConnectResultSet(df.collectResult())
+  }
+
+  // Schema of the returned DataFrame is:
+  // |-- TABLE_CAT: string (nullable = false)
+  // |-- TABLE_SCHEM: string (nullable = false)
+  // |-- TABLE_NAME: string (nullable = false)
+  // |-- TABLE_TYPE: string (nullable = false)
+  // |-- REMARKS: string (nullable = false)
+  // |-- TYPE_CAT: string (nullable = false)
+  // |-- TYPE_SCHEM: string (nullable = false)
+  // |-- TYPE_NAME: string (nullable = false)
+  // |-- SELF_REFERENCING_COL_NAME: string (nullable = false)
+  // |-- REF_GENERATION: string (nullable = false)
+  private def getTablesDataFrame(
+      catalog: String,
+      schemaPattern: String,
+      tableNamePattern: String): connect.DataFrame = {
+
+    val catalogSchemas = getSchemasDataFrame(catalog, schemaPattern).collect()
+      .map { row => (row.getString(1), row.getString(0)) }
+
+    val tableNameFilterExpr = if (isNullOrWildcard(tableNamePattern)) {
+      lit(true)
+    } else {
+      $"TABLE_NAME".like(tableNamePattern)
+    }
+
+    val emptyDf = conn.spark.emptyDataFrame
+      .withColumn("TABLE_CAT", lit(""))
+      .withColumn("TABLE_SCHEM", lit(""))
+      .withColumn("TABLE_NAME", lit(""))
+      .withColumn("TABLE_TYPE", lit(""))
+      .withColumn("REMARKS", lit(""))
+      .withColumn("TYPE_CAT", lit(""))
+      .withColumn("TYPE_SCHEM", lit(""))
+      .withColumn("TYPE_NAME", lit(""))
+      .withColumn("SELF_REFERENCING_COL_NAME", lit(""))
+      .withColumn("REF_GENERATION", lit(""))
+
+    catalogSchemas.map { case (catalog, schema) =>
+      val viewDf = try {
+        conn.spark
+          .sql(s"SHOW VIEWS IN ${quoteNameParts(Seq(catalog, schema))}")
+          .select($"namespace".as("TABLE_SCHEM"), $"viewName".as("TABLE_NAME"))
+          .filter(tableNameFilterExpr)
+      } catch {
+        case st: SparkThrowable if st.getCondition == "MISSING_CATALOG_ABILITY.VIEWS" =>

Review Comment:
   will revise `getSchemas` too after SPARK-54303 landing.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #52995:
URL: https://github.com/apache/spark/pull/52995#discussion_r2516512606


##########
sql/connect/client/jdbc/src/main/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaData.scala:
##########
@@ -355,20 +356,124 @@ class SparkConnectDatabaseMetaData(conn: SparkConnectConnection) extends Databas
   override def getSchemas(catalog: String, schemaPattern: String): ResultSet = {
     conn.checkOpen()
 
-    val df = getSchemasDataFrame(catalog, schemaPattern)
+    val df = getSchemasDataFrame(catalog, Some(schemaPattern))
       .orderBy("TABLE_CATALOG", "TABLE_SCHEM")
     new SparkConnectResultSet(df.collectResult())
   }
 
-  override def getTableTypes: ResultSet =
-    throw new SQLFeatureNotSupportedException
+  override def getTableTypes: ResultSet = {
+    conn.checkOpen()
+
+    val df = TABLE_TYPES.toDF("TABLE_TYPE")
+      .orderBy("TABLE_TYPE")
+    new SparkConnectResultSet(df.collectResult())
+  }
+
+  // Schema of the returned DataFrame is:
+  // |-- TABLE_CAT: string (nullable = false)
+  // |-- TABLE_SCHEM: string (nullable = false)
+  // |-- TABLE_NAME: string (nullable = false)
+  // |-- TABLE_TYPE: string (nullable = false)
+  // |-- REMARKS: string (nullable = false)
+  // |-- TYPE_CAT: string (nullable = false)
+  // |-- TYPE_SCHEM: string (nullable = false)
+  // |-- TYPE_NAME: string (nullable = false)
+  // |-- SELF_REFERENCING_COL_NAME: string (nullable = false)
+  // |-- REF_GENERATION: string (nullable = false)
+  private def getTablesDataFrame(
+      catalog: String,
+      schemaPattern: String,
+      tableNamePattern: String): connect.DataFrame = {
+
+    val catalogSchemasDf = if (schemaPattern == "") {

Review Comment:
   Can we ensure that the `schemaPattern` here is not `null`?



##########
sql/connect/client/jdbc/src/test/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaDataSuite.scala:
##########
@@ -343,4 +343,208 @@ class SparkConnectDatabaseMetaDataSuite extends ConnectFunSuite with RemoteSpark
       }
     }
   }
+
+  test("SparkConnectDatabaseMetaData getTableTypes") {
+    withConnection { conn =>
+      val metadata = conn.getMetaData
+      Using.resource(metadata.getTableTypes) { rs =>
+        val types = new Iterator[String] {
+          def hasNext: Boolean = rs.next()
+          def next(): String = rs.getString("TABLE_TYPE")
+        }.toSeq
+        // results are ordered by TABLE_TYPE
+        assert(types === Seq("TABLE", "VIEW"))
+      }
+    }
+  }
+
+  test("SparkConnectDatabaseMetaData getTables") {
+
+    case class GetTableResult(
+       TABLE_CAT: String,
+       TABLE_SCHEM: String,
+       TABLE_NAME: String,
+       TABLE_TYPE: String,
+       REMARKS: String,
+       TYPE_CAT: String,
+       TYPE_SCHEM: String,
+       TYPE_NAME: String,
+       SELF_REFERENCING_COL_NAME: String,
+       REF_GENERATION: String)
+
+    def verifyEmptyStringFields(result: GetTableResult): Unit = {
+      assert(result.REMARKS === "")
+      assert(result.TYPE_CAT === "")
+      assert(result.TYPE_SCHEM === "")
+      assert(result.TYPE_NAME === "")
+      assert(result.SELF_REFERENCING_COL_NAME === "")
+      assert(result.REF_GENERATION === "")
+    }
+
+    def verifyGetTables(
+        getTables: () => ResultSet)(verify: Seq[GetTableResult] => Unit): Unit = {
+      Using.resource(getTables()) { rs =>
+        val getTableResults = new Iterator[GetTableResult] {
+          def hasNext: Boolean = rs.next()
+          def next(): GetTableResult = GetTableResult(
+            TABLE_CAT = rs.getString("TABLE_CAT"),
+            TABLE_SCHEM = rs.getString("TABLE_SCHEM"),
+            TABLE_NAME = rs.getString("TABLE_NAME"),
+            TABLE_TYPE = rs.getString("TABLE_TYPE"),
+            REMARKS = rs.getString("REMARKS"),
+            TYPE_CAT = rs.getString("TYPE_CAT"),
+            TYPE_SCHEM = rs.getString("TYPE_SCHEM"),
+            TYPE_NAME = rs.getString("TYPE_NAME"),
+            SELF_REFERENCING_COL_NAME = rs.getString("SELF_REFERENCING_COL_NAME"),
+            REF_GENERATION = rs.getString("REF_GENERATION"))
+        }.toSeq
+        verify(getTableResults)
+      }
+    }
+
+    withConnection { conn =>
+      implicit val spark: SparkSession = conn.asInstanceOf[SparkConnectConnection].spark
+
+      // this catalog does not support view
+      registerCatalog("testcat", TEST_IN_MEMORY_CATALOG)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS testcat.t_db1")
+      spark.sql("CREATE TABLE IF NOT EXISTS testcat.t_db1.t_t1 (id INT)")
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db1")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db1.t1 (id INT)")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db1.t_2 (id INT)")
+      spark.sql(
+        """CREATE VIEW IF NOT EXISTS spark_catalog.db1.t1_v AS
+          |SELECT id FROM spark_catalog.db1.t1
+          |""".stripMargin)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db_2")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db_2.t_2 (id INT)")
+      spark.sql(
+        """CREATE VIEW IF NOT EXISTS spark_catalog.db_2.t_2_v AS
+          |SELECT id FROM spark_catalog.db_2.t_2
+          |""".stripMargin)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db_")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db_.t_ (id INT)")
+
+      val metadata = conn.getMetaData
+
+      // no need to care about "testcat" because it is memory based and session isolated,
+      // also is inaccessible from another SparkSession
+      withDatabase("spark_catalog.db1", "spark_catalog.db_2", "spark_catalog.db_") {
+        // list tables in all catalogs and schemas
+        val getTablesInAllCatalogsAndSchemas = List(null, "%").flatMap { database =>
+          List(null, "%").flatMap { table =>
+            List(null, Array("TABLE", "VIEW")).map { tableTypes =>
+              () => metadata.getTables(null, database, table, tableTypes)
+            }
+          }
+        }
+
+        getTablesInAllCatalogsAndSchemas.foreach { getTables =>
+          verifyGetTables(getTables) { getTableResults =>
+            // results are ordered by TABLE_TYPE, TABLE_CAT, TABLE_SCHEM and TABLE_NAME
+            assert {
+              getTableResults.map { result =>
+                (result.TABLE_TYPE, result.TABLE_CAT, result.TABLE_SCHEM, result.TABLE_NAME)
+              } === Seq(
+                ("TABLE", "spark_catalog", "db1", "t1"),
+                ("TABLE", "spark_catalog", "db1", "t_2"),
+                ("TABLE", "spark_catalog", "db_", "t_"),
+                ("TABLE", "spark_catalog", "db_2", "t_2"),
+                ("TABLE", "testcat", "t_db1", "t_t1"),
+                ("VIEW", "spark_catalog", "db1", "t1_v"),
+                ("VIEW", "spark_catalog", "db_2", "t_2_v"))
+            }
+            getTableResults.foreach(verifyEmptyStringFields)
+          }
+        }
+
+        // list tables with table types
+        val se = intercept[SQLException] {
+          metadata.getTables("spark_catalog", "foo", "bar", Array("TABLE", "METERIALIZED VIEW"))
+        }
+        assert(se.getMessage ===
+          "The requested table types contains unsupported items: METERIALIZED VIEW. " +

Review Comment:
   ditto



##########
sql/connect/client/jdbc/src/test/scala/org/apache/spark/sql/connect/client/jdbc/SparkConnectDatabaseMetaDataSuite.scala:
##########
@@ -343,4 +343,208 @@ class SparkConnectDatabaseMetaDataSuite extends ConnectFunSuite with RemoteSpark
       }
     }
   }
+
+  test("SparkConnectDatabaseMetaData getTableTypes") {
+    withConnection { conn =>
+      val metadata = conn.getMetaData
+      Using.resource(metadata.getTableTypes) { rs =>
+        val types = new Iterator[String] {
+          def hasNext: Boolean = rs.next()
+          def next(): String = rs.getString("TABLE_TYPE")
+        }.toSeq
+        // results are ordered by TABLE_TYPE
+        assert(types === Seq("TABLE", "VIEW"))
+      }
+    }
+  }
+
+  test("SparkConnectDatabaseMetaData getTables") {
+
+    case class GetTableResult(
+       TABLE_CAT: String,
+       TABLE_SCHEM: String,
+       TABLE_NAME: String,
+       TABLE_TYPE: String,
+       REMARKS: String,
+       TYPE_CAT: String,
+       TYPE_SCHEM: String,
+       TYPE_NAME: String,
+       SELF_REFERENCING_COL_NAME: String,
+       REF_GENERATION: String)
+
+    def verifyEmptyStringFields(result: GetTableResult): Unit = {
+      assert(result.REMARKS === "")
+      assert(result.TYPE_CAT === "")
+      assert(result.TYPE_SCHEM === "")
+      assert(result.TYPE_NAME === "")
+      assert(result.SELF_REFERENCING_COL_NAME === "")
+      assert(result.REF_GENERATION === "")
+    }
+
+    def verifyGetTables(
+        getTables: () => ResultSet)(verify: Seq[GetTableResult] => Unit): Unit = {
+      Using.resource(getTables()) { rs =>
+        val getTableResults = new Iterator[GetTableResult] {
+          def hasNext: Boolean = rs.next()
+          def next(): GetTableResult = GetTableResult(
+            TABLE_CAT = rs.getString("TABLE_CAT"),
+            TABLE_SCHEM = rs.getString("TABLE_SCHEM"),
+            TABLE_NAME = rs.getString("TABLE_NAME"),
+            TABLE_TYPE = rs.getString("TABLE_TYPE"),
+            REMARKS = rs.getString("REMARKS"),
+            TYPE_CAT = rs.getString("TYPE_CAT"),
+            TYPE_SCHEM = rs.getString("TYPE_SCHEM"),
+            TYPE_NAME = rs.getString("TYPE_NAME"),
+            SELF_REFERENCING_COL_NAME = rs.getString("SELF_REFERENCING_COL_NAME"),
+            REF_GENERATION = rs.getString("REF_GENERATION"))
+        }.toSeq
+        verify(getTableResults)
+      }
+    }
+
+    withConnection { conn =>
+      implicit val spark: SparkSession = conn.asInstanceOf[SparkConnectConnection].spark
+
+      // this catalog does not support view
+      registerCatalog("testcat", TEST_IN_MEMORY_CATALOG)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS testcat.t_db1")
+      spark.sql("CREATE TABLE IF NOT EXISTS testcat.t_db1.t_t1 (id INT)")
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db1")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db1.t1 (id INT)")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db1.t_2 (id INT)")
+      spark.sql(
+        """CREATE VIEW IF NOT EXISTS spark_catalog.db1.t1_v AS
+          |SELECT id FROM spark_catalog.db1.t1
+          |""".stripMargin)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db_2")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db_2.t_2 (id INT)")
+      spark.sql(
+        """CREATE VIEW IF NOT EXISTS spark_catalog.db_2.t_2_v AS
+          |SELECT id FROM spark_catalog.db_2.t_2
+          |""".stripMargin)
+
+      spark.sql("CREATE DATABASE IF NOT EXISTS spark_catalog.db_")
+      spark.sql("CREATE TABLE IF NOT EXISTS spark_catalog.db_.t_ (id INT)")
+
+      val metadata = conn.getMetaData
+
+      // no need to care about "testcat" because it is memory based and session isolated,
+      // also is inaccessible from another SparkSession
+      withDatabase("spark_catalog.db1", "spark_catalog.db_2", "spark_catalog.db_") {
+        // list tables in all catalogs and schemas
+        val getTablesInAllCatalogsAndSchemas = List(null, "%").flatMap { database =>
+          List(null, "%").flatMap { table =>
+            List(null, Array("TABLE", "VIEW")).map { tableTypes =>
+              () => metadata.getTables(null, database, table, tableTypes)
+            }
+          }
+        }
+
+        getTablesInAllCatalogsAndSchemas.foreach { getTables =>
+          verifyGetTables(getTables) { getTableResults =>
+            // results are ordered by TABLE_TYPE, TABLE_CAT, TABLE_SCHEM and TABLE_NAME
+            assert {
+              getTableResults.map { result =>
+                (result.TABLE_TYPE, result.TABLE_CAT, result.TABLE_SCHEM, result.TABLE_NAME)
+              } === Seq(
+                ("TABLE", "spark_catalog", "db1", "t1"),
+                ("TABLE", "spark_catalog", "db1", "t_2"),
+                ("TABLE", "spark_catalog", "db_", "t_"),
+                ("TABLE", "spark_catalog", "db_2", "t_2"),
+                ("TABLE", "testcat", "t_db1", "t_t1"),
+                ("VIEW", "spark_catalog", "db1", "t1_v"),
+                ("VIEW", "spark_catalog", "db_2", "t_2_v"))
+            }
+            getTableResults.foreach(verifyEmptyStringFields)
+          }
+        }
+
+        // list tables with table types
+        val se = intercept[SQLException] {
+          metadata.getTables("spark_catalog", "foo", "bar", Array("TABLE", "METERIALIZED VIEW"))

Review Comment:
   MATERIALIZED ?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-54113][CONNECT] Support getTables for SparkConnectDatabaseMetaData [spark]

Posted by "dongjoon-hyun (via GitHub)" <gi...@apache.org>.
dongjoon-hyun commented on PR #52995:
URL: https://github.com/apache/spark/pull/52995#issuecomment-3520072687

   Merged to master to unblock the remaining feature PRs. We have enough time to do the follow-up PRs and bug fixes after the mid of November.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org