You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "ayushbilala (via GitHub)" <gi...@apache.org> on 2026/03/16 11:46:35 UTC
[PR] [SPARK-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
ayushbilala opened a new pull request, #54824:
URL: https://github.com/apache/spark/pull/54824
### What changes were proposed in this pull request?
Support SHOW TABLES ... [AS JSON] and SHOW TABLE EXTENDED ... [AS JSON] to optionally display table listing metadata in JSON format.
SQL syntax:
```
SHOW TABLES [(IN|FROM) database_name] [[LIKE] pattern] [AS JSON]
SHOW TABLE EXTENDED [(IN|FROM) database_name] LIKE pattern [AS JSON]
```
Output: json_metadata: String
SHOW TABLES AS JSON:
`{"tables":[{"name":"t1","namespace":["db"],"isTemporary":false}]}`
SHOW TABLE EXTENDED ... AS JSON additionally includes catalog and type:
`{"tables":[{"catalog":"spark_catalog","namespace":["db"],"name":"t1","type":"TABLE","isTemporary":false}]}`
### Why are the changes needed?
The existing text-based output of SHOW TABLES requires fragile string parsing for programmatic consumption.
A structured JSON format provides a stable, machine-readable contract for tooling and automation.
### Does this PR introduce _any_ user-facing change?
Yes. Two new SQL syntax variants that return JSON output. Existing commands without AS JSON are unaffected.
SHOW TABLE EXTENDED with both PARTITION and AS JSON is explicitly rejected.
### How was this patch tested?
- Parser tests in `ShowTablesParserSuite` for all AS JSON variants and the PARTITION + AS JSON error case.
- Execution tests in `ShowTablesSuiteBase` covering JSON schema validation, empty databases, EXTENDED output, and temp view inclusion.
- Manual verification in spark-shell
### Was this patch authored or co-authored using generative AI tooling?
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3208576876
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowTablesSuiteBase.scala:
##########
@@ -461,4 +466,179 @@ trait ShowTablesSuiteBase extends QueryTest with DDLCommandTestUtils {
}
}
}
+
+ test("SHOW TABLES AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ val tblEntry = tables.find(t => (t \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON with empty database") {
+ withNamespace(s"$catalog.ns_empty") {
+ sql(s"CREATE NAMESPACE $catalog.ns_empty")
+ val df = sql(s"SHOW TABLES IN $catalog.ns_empty AS JSON")
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.isEmpty)
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1)
+ val entry = tables.head
+ assert((entry \ "name").extract[String] == "tbl")
+ assert((entry \ "type").extract[String] == "TABLE")
+ assert((entry \ "isTemporary").extract[Boolean] == false)
+ assert((entry \ "catalog").isInstanceOf[JString])
+ assert((entry \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON includes temp views") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ withTempView("tv") {
+ sql("CREATE TEMP VIEW tv AS SELECT 1 AS id")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ val tempView = tables.find(t => (t \ "name").extract[String] == "tv")
+ assert(tempView.isDefined)
+ assert((tempView.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with local temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val localTmpViewName = "tbl_local_tmp"
+ withTempView(localTmpViewName) {
+ sql(s"CREATE TEMPORARY VIEW $localTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl*' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ val tblEntry = tables.find(e => (e \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "type").extract[String] == "TABLE")
+
+ val tempViewEntry = tables.find(e => (e \ "name").extract[String] == localTmpViewName)
+ assert(tempViewEntry.isDefined)
+ assert((tempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ assert((tempViewEntry.get \ "type").extract[String] == "VIEW")
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with global temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val globalTmpViewName = "ext_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $globalNamespace LIKE 'ext_json*' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ val globalTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == globalTmpViewName)
+ assert(globalTempViewEntry.isDefined)
+ assert((globalTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ assert((globalTempViewEntry.get \ "type").extract[String] == "VIEW")
+ }
+ }
+ }
+
+ test("SHOW TABLES AS JSON with global temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val globalTmpViewName = "show_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLES IN $globalNamespace AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ val globalTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == globalTmpViewName)
+ assert(globalTempViewEntry.isDefined)
+ assert((globalTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with both local and global temp views") {
Review Comment:
Fixed - added assert(tables.length == N) to the tests that used .find() without a count assertion. This will catch silent duplicates.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3208561336
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesExtendedExec.scala:
##########
@@ -73,6 +85,46 @@ case class ShowTablesExtendedExec(
rows.toSeq
}
+ private def runAsJson(): Seq[InternalRow] = {
+ val jsonRows = new ArrayBuffer[JObject]()
+
+ // fetch tables
+ val tables = catalog.listTables(namespace.toArray)
+ tables.foreach { tableIdent =>
+ if (StringUtils.filterPattern(Seq(tableIdent.name()), pattern).nonEmpty) {
+ // V2 persistent tables are always TABLE
+ jsonRows += JObject(
+ "catalog" -> JString(catalog.name()),
+ "namespace" -> JArray(tableIdent.namespace().map(JString(_)).toList),
+ "name" -> JString(tableIdent.name()),
+ "type" -> JString("TABLE"),
+ "isTemporary" -> JBool(false)
+ )
+ }
+ }
+
+ // fetch temp views, includes: global temp view, local temp view
+ val sessionCatalog = session.sessionState.catalog
+ val db = namespace match {
+ case Seq(db) => Some(db)
+ case _ => None
+ }
+ val tempViews = sessionCatalog.listTempViews(db.getOrElse(""), pattern)
Review Comment:
Fixed. JSON path has been extracted to ShowTablesJsonExec. There is a guard !CatalogV2Util.isSessionCatalog(catalog) to prevent the duplicate listTempViews() fetch when catalog is V2SessionCatalog.
Remaining code in ShowTablesExtendedExec is the text-only path, which is unchanged.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3357152582
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala:
##########
@@ -25,7 +25,6 @@ import scala.util.control.NonFatal
import org.apache.hadoop.fs.{FileContext, FsConstants, Path}
import org.apache.hadoop.fs.permission.{AclEntry, AclEntryScope, AclEntryType, FsAction, FsPermission}
-
import org.apache.spark.sql.{Row, SparkSession}
Review Comment:
This drops the blank line separating the 3rd-party import group (`org.apache.hadoop.*`) from the `spark` group, which fails scalastyle's `ImportOrderChecker`:
```
There should at least one a single empty line separating groups 3rdParty and spark. line=28
```
so the lint CI gate will be red. This blank-line removal is in fact this file's *only* change versus `master` (the `runAsJson` code that used to be here was never on `master`), so restoring the blank line reverts the file entirely:
```suggestion
import org.apache.spark.sql.{Row, SparkSession}
```
Minor related note: `AstBuilder`, `ResolveSessionCatalog`, `DataSourceV2Strategy`, and `ShowTablesExec` also carry leftover residue from the `asJson` refactor (formatting collapses / a behavior-preserving filter+emit split) with no net functional change — harmless, but reverting the purely cosmetic ones would keep the diff minimal.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ShowTablesJsonCommand.scala:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command
+
+import scala.collection.mutable.ArrayBuffer
+
+import org.json4s._
+import org.json4s.jackson.JsonMethods._
+
+import org.apache.spark.sql.{Row, SparkSession}
+import org.apache.spark.sql.catalyst.analysis.ResolvedNamespace
+import org.apache.spark.sql.catalyst.catalog.CatalogTableType
+import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.util.StringUtils
+import org.apache.spark.sql.connector.catalog.{CatalogV2Util, TableCatalog, TableViewCatalog}
+import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._
+import org.apache.spark.sql.types.{MetadataBuilder, StringType}
+
+/**
+ * The command for `SHOW TABLES AS JSON` and `SHOW TABLE EXTENDED AS JSON`.
+ */
+case class ShowTablesJsonCommand(
+ child: LogicalPlan,
+ pattern: Option[String],
+ isExtended: Boolean,
+ override val output: Seq[Attribute] = Seq(
+ AttributeReference(
+ "json_metadata",
+ StringType,
+ nullable = false,
+ new MetadataBuilder()
+ .putString("comment", "JSON metadata of the tables")
+ .build())()
+ )) extends UnaryRunnableCommand {
+
+ override def run(sparkSession: SparkSession): Seq[Row] = {
+ val jsonOutput = child match {
+ case ResolvedNamespace(catalog, ns, _) =>
Review Comment:
(non-blocking) This `match` handles only `ResolvedNamespace`, so any other child would surface as a cryptic `scala.MatchError`. It's unreachable today — the analyzer always resolves the namespace child to `ResolvedNamespace` — but the analogue this command is modeled on, `DescribeRelationJsonCommand`, uses an explicit `case _ => throw …`. Consider an explicit `case _` (internal error) so the failure is clear if that invariant ever changes.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ShowTablesJsonCommand.scala:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command
+
+import scala.collection.mutable.ArrayBuffer
+
+import org.json4s._
+import org.json4s.jackson.JsonMethods._
+
+import org.apache.spark.sql.{Row, SparkSession}
+import org.apache.spark.sql.catalyst.analysis.ResolvedNamespace
+import org.apache.spark.sql.catalyst.catalog.CatalogTableType
+import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.util.StringUtils
+import org.apache.spark.sql.connector.catalog.{CatalogV2Util, TableCatalog, TableViewCatalog}
+import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._
+import org.apache.spark.sql.types.{MetadataBuilder, StringType}
+
+/**
+ * The command for `SHOW TABLES AS JSON` and `SHOW TABLE EXTENDED AS JSON`.
+ */
+case class ShowTablesJsonCommand(
+ child: LogicalPlan,
+ pattern: Option[String],
+ isExtended: Boolean,
+ override val output: Seq[Attribute] = Seq(
+ AttributeReference(
+ "json_metadata",
+ StringType,
+ nullable = false,
+ new MetadataBuilder()
+ .putString("comment", "JSON metadata of the tables")
+ .build())()
+ )) extends UnaryRunnableCommand {
+
+ override def run(sparkSession: SparkSession): Seq[Row] = {
+ val jsonOutput = child match {
+ case ResolvedNamespace(catalog, ns, _) =>
+ if (CatalogV2Util.isSessionCatalog(catalog)) {
+ runForSessionCatalog(sparkSession, ns)
+ } else {
+ runForV2Catalog(sparkSession, catalog.asTableCatalog, ns)
+ }
+ }
+ Seq(Row(compact(render(jsonOutput))))
+ }
+
+ private def runForSessionCatalog(
+ sparkSession: SparkSession,
+ ns: Seq[String]): JObject = {
+ val sessionCatalog = sparkSession.sessionState.catalog
+ val db = ns.headOption.getOrElse(sessionCatalog.getCurrentDatabase)
+ val tables = pattern
+ .map(p => sessionCatalog.listTables(db, p))
+ .getOrElse(sessionCatalog.listTables(db))
+
+ val jsonTables = tables.map { tableIdent =>
+ val isTemp = sessionCatalog.isTempView(tableIdent)
+ val namespace = tableIdent.database.toList
+
+ if (isExtended) {
+ val tableType = if (isTemp) {
+ "VIEW"
+ } else {
+ val meta = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
+ if (meta.tableType == CatalogTableType.VIEW) "VIEW" else "TABLE"
Review Comment:
Two issues with the extended `type` field, both fixable here:
1. **`METRIC_VIEW` is misclassified as `TABLE`.** This checks `meta.tableType == CatalogTableType.VIEW`, but `CatalogTable.isViewLike` is `VIEW || METRIC_VIEW` (`interface.scala:784-786`). A persistent metric view falls through to `"TABLE"` — a silent wrong value. Minimal fix: use `meta.isViewLike`.
2. **The `TABLE`/`VIEW` vocabulary is coarser than the rest of the system.** The sibling `DescribeRelationJsonCommand` emits the full `tableType.name` (`MANAGED`/`EXTERNAL`/`VIEW`/`METRIC_VIEW`, `interface.scala:670`), and the text `SHOW TABLE EXTENDED` exposes the same granular `Type`. Since the JSON output is meant to be a *stable machine-readable contract*, collapsing to `TABLE`/`VIEW` drops the `MANAGED`-vs-`EXTERNAL` distinction and diverges from the `DESCRIBE AS JSON` sibling — and it's expensive to change once the contract ships. Emitting `meta.tableType.name` here would fix the metric-view bug and the inconsistency in one go. Is the coarse vocabulary intentional, or worth aligning with `DESCRIBE AS JSON`?
(The V2 path at line 135 always emits `TABLE`; that's defensible since only `catalog.listTables` rows are listed there and avoiding a per-table `loadTable` is a reasonable perf choice — it just shares the same vocabulary question.)
This predates this round (the old `ShowTablesJsonExec` had `if (isTemporary) "VIEW" else "TABLE"`), so it's a late catch on my side — not something this refactor introduced.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4663412479
Reopening from a named branch to fix CI.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4655891114
can you check your github action settings? CI has not been triggered for this PR.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4656807908
> can you check your github action settings? CI has not been triggered for this PR.
@cloud-fan CI skip is caused by [SPARK-57074 ](https://github.com/apache/spark/pull/56117)(merged May 27), which added a condition to build_main.yml that skips the Build workflow on fork master pushes. Since this PR's head branch is master on my fork, the push-triggered CI is being skipped.
I've verified all tests locally, both new and existing ShowTables* suites pass. Would it be possible to re-trigger CI from the upstream side, or should I close this PR and reopen from a named branch?
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4655891924
otherwise LGTM
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3362670138
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ShowTablesJsonCommand.scala:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command
+
+import scala.collection.mutable.ArrayBuffer
+
+import org.json4s._
+import org.json4s.jackson.JsonMethods._
+
+import org.apache.spark.sql.{Row, SparkSession}
+import org.apache.spark.sql.catalyst.analysis.ResolvedNamespace
+import org.apache.spark.sql.catalyst.catalog.CatalogTableType
+import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.util.StringUtils
+import org.apache.spark.sql.connector.catalog.{CatalogV2Util, TableCatalog, TableViewCatalog}
+import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._
+import org.apache.spark.sql.types.{MetadataBuilder, StringType}
+
+/**
+ * The command for `SHOW TABLES AS JSON` and `SHOW TABLE EXTENDED AS JSON`.
+ */
+case class ShowTablesJsonCommand(
+ child: LogicalPlan,
+ pattern: Option[String],
+ isExtended: Boolean,
+ override val output: Seq[Attribute] = Seq(
+ AttributeReference(
+ "json_metadata",
+ StringType,
+ nullable = false,
+ new MetadataBuilder()
+ .putString("comment", "JSON metadata of the tables")
+ .build())()
+ )) extends UnaryRunnableCommand {
+
+ override def run(sparkSession: SparkSession): Seq[Row] = {
+ val jsonOutput = child match {
+ case ResolvedNamespace(catalog, ns, _) =>
+ if (CatalogV2Util.isSessionCatalog(catalog)) {
+ runForSessionCatalog(sparkSession, ns)
+ } else {
+ runForV2Catalog(sparkSession, catalog.asTableCatalog, ns)
+ }
+ }
+ Seq(Row(compact(render(jsonOutput))))
+ }
+
+ private def runForSessionCatalog(
+ sparkSession: SparkSession,
+ ns: Seq[String]): JObject = {
+ val sessionCatalog = sparkSession.sessionState.catalog
+ val db = ns.headOption.getOrElse(sessionCatalog.getCurrentDatabase)
+ val tables = pattern
+ .map(p => sessionCatalog.listTables(db, p))
+ .getOrElse(sessionCatalog.listTables(db))
+
+ val jsonTables = tables.map { tableIdent =>
+ val isTemp = sessionCatalog.isTempView(tableIdent)
+ val namespace = tableIdent.database.toList
+
+ if (isExtended) {
+ val tableType = if (isTemp) {
+ "VIEW"
+ } else {
+ val meta = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
+ if (meta.tableType == CatalogTableType.VIEW) "VIEW" else "TABLE"
Review Comment:
Good catch. The coarse `TABLE/VIEW` was carried over from the old `ShowTablesJsonExec` and is not an intentional design choice. Will switch to `meta.tableType.name` to align with `DESCRIBE AS JSON `(which emits `MANAGED/EXTERNAL/VIEW/METRIC_VIEW` via `CatalogTable.toLinkedHashMap)`.
For the V2 path (line 135), will keep `TABLE` since resolving the actual type would require a per-table loadTable call
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala closed pull request #54824: [SPARK-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED
URL: https://github.com/apache/spark/pull/54824
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3335890494
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala:
##########
@@ -972,6 +986,45 @@ case class ShowTablesCommand(
Seq(Row(database, tableName, isTemp, s"$information\n"))
}
}
+
+ private def runAsJson(sparkSession: SparkSession): Seq[Row] = {
Review Comment:
Got it. We can create a new `ShowTablesJsonCommand` modeled on `DescribeRelationJsonCommand`.
Since `ShowTablesJsonCommand` must live in `sql/core`, we override `visitShowTables`/`visitShowTableExtended` in `SparkSqlAstBuilder` (not `AstBuilder`) to produce it when `AS JSON` is present. Inside` run()`, we dispatch on `CatalogV2Util.isSessionCatalog ` calling `SessionCatalog.listTables` for the session catalog and `catalog.listTables` for V2 catalogs.
This eliminates both `ShowTablesCommand.runAsJson()` and `ShowTablesJsonExec`.
Does this align with what you had in mind?
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4405723357
@cloud-fan Thanks for the detailed review.
On duplication / unified command: I opted to fix the underlying bugs in place rather than introduce a new command class, to keep the PR focused. Both bugs you identified are now fixed:
- V2 temp view listing: ShowTablesExec guards listTempViews() with !catalog.isInstanceOf[TableViewCatalog] && !CatalogV2Util.isSessionCatalog(catalog) avoiding duplicates for both session-catalog and TableViewCatalog cases. ShowTablesExtendedExec uses !CatalogV2Util.isSessionCatalog(catalog).
- V1 catalog name: ShowTablesCommand now uses sparkSession.sessionState.catalogManager.v2SessionCatalog.name().
If you'd prefer the unified approach, I'll be happy to restructure.
On duplicate detection in tests: Added assert(names.distinct.length == names.length, ...) to the two tests most likely to surface duplicates: "SHOW TABLES AS JSON includes temp views" and "SHOW TABLE EXTENDED AS JSON with local temp view".
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3208603940
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesExec.scala:
##########
@@ -33,18 +36,32 @@ case class ShowTablesExec(
output: Seq[Attribute],
catalog: TableCatalog,
namespace: Seq[String],
- pattern: Option[String]) extends V2CommandExec with LeafExecNode {
+ pattern: Option[String],
+ asJson: Boolean = false) extends V2CommandExec with LeafExecNode {
override protected def run(): Seq[InternalRow] = {
- val rows = new ArrayBuffer[InternalRow]()
-
val tables = catalog.listTables(namespace.toArray)
- tables.map { table =>
- if (pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)) {
+ val filteredTables = tables.filter { table =>
+ pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)
+ }
+
+ if (asJson) {
+ val jsonTables = filteredTables.map { table =>
+ JObject(
+ "name" -> JString(table.name()),
Review Comment:
Fixed. JSON code is consolidated in ShowTablesJsonExec.toJsonEntry(). Both schemas now have name as the first field: non-extended is {name, namespace, isTemporary} and extended is {name, catalog, namespace, type, isTemporary}.
V1 path in ShowTablesCommand follows the same ordering.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4662853854
please re-create the pr, passing CI is a hard requirement to merge PRs
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3364403653
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowTablesSuiteBase.scala:
##########
@@ -461,4 +467,237 @@ trait ShowTablesSuiteBase extends QueryTest with DDLCommandTestUtils {
}
}
}
+
+ test("SHOW TABLES AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1, s"Expected 1 entry, got: $tables")
+ val tblEntry = tables.find(t => (t \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON with empty database") {
+ withNamespace(s"$catalog.ns_empty") {
+ sql(s"CREATE NAMESPACE $catalog.ns_empty")
+ val df = sql(s"SHOW TABLES IN $catalog.ns_empty AS JSON")
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.isEmpty)
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1)
+ val entry = tables.head
+ assert((entry \ "name").extract[String] == "tbl")
+ assert((entry \ "type").extract[String] == expectedTableTypeInJson)
+ assert((entry \ "isTemporary").extract[Boolean] == false)
+ assert((entry \ "catalog").isInstanceOf[JString])
+ assert((entry \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON includes temp views") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ withTempView("tv") {
+ sql("CREATE TEMP VIEW tv AS SELECT 1 AS id")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ val names = tables.map(t => (t \ "name").extract[String])
+ assert(names.distinct.length == names.length, s"Duplicate entries found: $names")
+ val tempView = tables.find(t => (t \ "name").extract[String] == "tv")
+ assert(tempView.isDefined)
+ assert((tempView.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with local temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val localTmpViewName = "tbl_local_tmp"
+ withTempView(localTmpViewName) {
+ sql(s"CREATE TEMPORARY VIEW $localTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl*' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 2, s"Expected 2 entries (tbl + $localTmpViewName), got: $tables")
Review Comment:
**This test fails when run by the V2 `ShowTablesSuite`.** `ShowTablesSuiteBase` is shared by both the V1 and V2 concrete suites. Here `localTmpViewName` is a *session-local* temp view, but the query is `SHOW TABLE EXTENDED IN $catalog.ns ... AS JSON`. In the V2 suite `$catalog` is `test_catalog` (a non-session `InMemoryPartitionTableCatalog`), so resolution routes to `ShowTablesJsonCommand.runForV2Catalog`, which — correctly, per the agreed design (your reply to @asl3 on r3301795477) — does not fetch session temp views. So only `tbl` is listed and `tables.length == 2` fails on the V2 path. (The `global_temp` variants pass because `IN global_temp` resolves to the session catalog even in the V2 suite.)
Suggest making session-temp-view presence catalog-aware — e.g. an `expectsSessionTempViews` override hook mirroring the new `expectedTableTypeInJson`, true only for the session/V1 path — or scoping this test (and the one at :533) to the V1 suite. Confirmed by running `command/v2/ShowTablesSuite` locally (12 run, 10 passed, these 2 failed); it's latent rather than red only because the fork's test matrix never ran on this PR (all runs `skipped`).
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowTablesSuiteBase.scala:
##########
@@ -461,4 +467,237 @@ trait ShowTablesSuiteBase extends QueryTest with DDLCommandTestUtils {
}
}
}
+
+ test("SHOW TABLES AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1, s"Expected 1 entry, got: $tables")
+ val tblEntry = tables.find(t => (t \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON with empty database") {
+ withNamespace(s"$catalog.ns_empty") {
+ sql(s"CREATE NAMESPACE $catalog.ns_empty")
+ val df = sql(s"SHOW TABLES IN $catalog.ns_empty AS JSON")
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.isEmpty)
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1)
+ val entry = tables.head
+ assert((entry \ "name").extract[String] == "tbl")
+ assert((entry \ "type").extract[String] == expectedTableTypeInJson)
+ assert((entry \ "isTemporary").extract[Boolean] == false)
+ assert((entry \ "catalog").isInstanceOf[JString])
+ assert((entry \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON includes temp views") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ withTempView("tv") {
+ sql("CREATE TEMP VIEW tv AS SELECT 1 AS id")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ val names = tables.map(t => (t \ "name").extract[String])
+ assert(names.distinct.length == names.length, s"Duplicate entries found: $names")
+ val tempView = tables.find(t => (t \ "name").extract[String] == "tv")
Review Comment:
Same root cause as the comment on line 554: `tv` is a session-local temp view, so it is absent from the V2 catalog's listing (`runForV2Catalog` does not fetch session temp views), and this `assert(tempView.isDefined)` fails when this test runs in the V2 `ShowTablesSuite`.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowTablesSuiteBase.scala:
##########
@@ -461,4 +467,237 @@ trait ShowTablesSuiteBase extends QueryTest with DDLCommandTestUtils {
}
}
}
+
+ test("SHOW TABLES AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1, s"Expected 1 entry, got: $tables")
+ val tblEntry = tables.find(t => (t \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON with empty database") {
+ withNamespace(s"$catalog.ns_empty") {
+ sql(s"CREATE NAMESPACE $catalog.ns_empty")
+ val df = sql(s"SHOW TABLES IN $catalog.ns_empty AS JSON")
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.isEmpty)
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1)
+ val entry = tables.head
+ assert((entry \ "name").extract[String] == "tbl")
+ assert((entry \ "type").extract[String] == expectedTableTypeInJson)
+ assert((entry \ "isTemporary").extract[Boolean] == false)
+ assert((entry \ "catalog").isInstanceOf[JString])
+ assert((entry \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON includes temp views") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ withTempView("tv") {
+ sql("CREATE TEMP VIEW tv AS SELECT 1 AS id")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ val names = tables.map(t => (t \ "name").extract[String])
+ assert(names.distinct.length == names.length, s"Duplicate entries found: $names")
+ val tempView = tables.find(t => (t \ "name").extract[String] == "tv")
+ assert(tempView.isDefined)
+ assert((tempView.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with local temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val localTmpViewName = "tbl_local_tmp"
+ withTempView(localTmpViewName) {
+ sql(s"CREATE TEMPORARY VIEW $localTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl*' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 2, s"Expected 2 entries (tbl + $localTmpViewName), got: $tables")
+
+ val names = tables.map(e => (e \ "name").extract[String])
+ assert(names.distinct.length == names.length, s"Duplicate entries found: $names")
+
+ val tblEntry = tables.find(e => (e \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "type").extract[String] == expectedTableTypeInJson)
+
+ val tempViewEntry = tables.find(e => (e \ "name").extract[String] == localTmpViewName)
+ assert(tempViewEntry.isDefined)
+ assert((tempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ assert((tempViewEntry.get \ "type").extract[String] == "VIEW")
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with global temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val globalTmpViewName = "ext_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $globalNamespace LIKE 'ext_json*' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1, s"Expected 1 entry, got: $tables")
+
+ val globalTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == globalTmpViewName)
+ assert(globalTempViewEntry.isDefined)
+ assert((globalTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ assert((globalTempViewEntry.get \ "type").extract[String] == "VIEW")
+ }
+ }
+ }
+
+ test("SHOW TABLES AS JSON with global temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val globalTmpViewName = "show_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLES IN $globalNamespace AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1, s"Expected 1 entry, got: $tables")
+
+ val globalTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == globalTmpViewName)
+ assert(globalTempViewEntry.isDefined)
+ assert((globalTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with both local and global temp views") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val localTmpViewName = "both_json_ltv"
+ val globalTmpViewName = "both_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(localTmpViewName, s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE TEMP VIEW $localTmpViewName AS SELECT id FROM $t")
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $globalNamespace LIKE 'both_json*' AS JSON")
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ assert(tables.length == 2)
+
+ val globalTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == globalTmpViewName)
+ assert(globalTempViewEntry.isDefined)
+ assert((globalTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+
+ val localTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == localTmpViewName)
+ assert(localTempViewEntry.isDefined)
+ assert((localTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLES AS JSON with LIKE pattern") {
+ withNamespaceAndTable("ns", "tab_matched") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ withTable(s"$catalog.ns.other") {
+ sql(s"CREATE TABLE $catalog.ns.other (id INT) $defaultUsing")
+ val jsonStr = sql(s"SHOW TABLES IN $catalog.ns LIKE 'tab_matched' AS JSON")
+ .collect()(0).getString(0)
+ val tables = (parse(jsonStr) \ "tables").asInstanceOf[JArray].arr
+ assert(tables.exists(e => (e \ "name").extract[String] == "tab_matched"))
+ assert(!tables.exists(e => (e \ "name").extract[String] == "other"))
+ }
+ }
+ }
+
+ test("SHOW TABLES AS JSON without IN clause") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ sql(s"USE $catalog.ns")
+ val jsonStr = sql("SHOW TABLES AS JSON").collect()(0).getString(0)
+ val tables = (parse(jsonStr) \ "tables").asInstanceOf[JArray].arr
+ assert(tables.exists(e => (e \ "name").extract[String] == "tbl"))
+ }
+ }
+
+ test("SHOW TABLES AS JSON wraps entries in tables key") {
+ withNamespace(s"$catalog.ns") {
+ sql(s"CREATE NAMESPACE $catalog.ns")
+ val jsonStr = sql(s"SHOW TABLES IN $catalog.ns AS JSON").collect()(0).getString(0)
+ val json = parse(jsonStr)
+ assert(json \ "tables" != JNothing)
+ }
+ }
+
+ test("SHOW TABLES AS JSON with useV1Command") {
Review Comment:
(nit) The body asserts `namesDefault === namesV1` — a parity check across `useV1Command` true/false — but the renamed title no longer conveys that.
```suggestion
test("SHOW TABLES AS JSON produces same result for useV1Command true and false") {
```
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3208030348
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala:
##########
@@ -970,6 +982,48 @@ case class ShowTablesCommand(
Seq(Row(database, tableName, isTemp, s"$information\n"))
}
}
+
+ private def runAsJson(sparkSession: SparkSession): Seq[Row] = {
+ val catalog = sparkSession.sessionState.catalog
+ val db = databaseName.getOrElse(catalog.getCurrentDatabase)
+ val tables =
+ tableIdentifierPattern.map(catalog.listTables(db, _)).getOrElse(catalog.listTables(db))
+
+ val jsonTables = tables.map { tableIdent =>
+ val isTemp = catalog.isTempView(tableIdent)
+ val ns = tableIdent.database.toList
+
+ if (isExtended) {
+ val tableType = if (isTemp) {
+ "VIEW"
+ } else {
+ Try(catalog.getTempViewOrPermanentTableMetadata(tableIdent)) match {
+ case Success(meta) if meta.tableType == CatalogTableType.VIEW => "VIEW"
+ case Success(_) => "TABLE"
+ case Failure(_) => "UNKNOWN"
Review Comment:
Fixed - Try/catch block that returned "UNKNOWN" on failure has been removed. getTempViewOrPermanentTableMetadata now propagates exceptions directly, consistent with the text path.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4406298002
@cloud-fan Both addressed.
For (a): we extracted a single ShowTablesJsonExec physical plan node that handles both SHOW TABLES AS JSON (isExtended=false) and SHOW TABLE EXTENDED AS JSON (isExtended=true).
DataSourceV2Strategy now routes asJson=true to it for both variants, eliminating the duplicated JSON emission from ShowTablesExec and ShowTablesExtendedExec. V1 path in ShowTablesCommand remains separate since it operates on a different plan type (ResolvedV1Database).
For (b): added assert(tables.length == N) to the tests that used .find() without a count check.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4283301405
Addressed all review comments. Please take another pass when you get a chance. Thanks!
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4160927952
@asl3 I have added tests for temp views. Can you please check?
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "asl3 (via GitHub)" <gi...@apache.org>.
asl3 commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3301773915
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala:
##########
@@ -1371,17 +1371,22 @@ case class RenameTable(
case class ShowTables(
namespace: LogicalPlan,
pattern: Option[String],
+ asJson: Boolean = false,
override val output: Seq[Attribute] = ShowTables.getOutputAttrs) extends UnaryCommand {
override def child: LogicalPlan = namespace
override protected def withNewChildInternal(newChild: LogicalPlan): ShowTables =
copy(namespace = newChild)
+ override protected def stringArgs: Iterator[Any] = Iterator(pattern, output)
Review Comment:
The `asJson: Boolean = false` field added to `ShowTables` (and
the parallel field on `ShowTablesExtended` and `ShowTablesCommand`) is then
immediately hidden from `treeString` by an `override protected def stringArgs:
Iterator[Any] = Iterator(pattern, output)` further down. `SHOW TABLES`
(3-column schema) and `SHOW TABLES AS JSON` (1-column `json_metadata` schema)
produce different output schemas; their `treeString` should differ. The
override exists to keep golden files passing, but that hides a real plan-level
distinction.
**Suggestion:** Drop the `stringArgs` overrides and regenerate the affected
explain/golden files. If a specific suite needs the legacy string, scope the
override narrowly to that suite's plan view rather than to the case class.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowTablesSuiteBase.scala:
##########
@@ -461,4 +466,190 @@ trait ShowTablesSuiteBase extends QueryTest with DDLCommandTestUtils {
}
}
}
+
+ test("SHOW TABLES AS JSON returns single row with json_metadata column") {
Review Comment:
Seems tests cover happy paths only. The following code paths added by this PR are not exercised -- can you add test coverage for the below?
1. **Pattern-LIKE exclusion.** Both V1 (`tableIdentifierPattern`) and V2
(`StringUtils.filterPattern`) run pattern filtering, but every existing filter
test uses a pattern where all tables match. No test creates `foo1` and `bar1`
and asserts `SHOW TABLES IN ns LIKE 'foo*' AS JSON` emits only `foo1`.
2. **Current-namespace `SHOW TABLES AS JSON`** (no `IN`). All tests use `IN
$catalog.ns`. The grammar allows `SHOW TABLES AS JSON` resolving to
`CurrentNamespace` (`AstBuilder.scala:5798`).
3. **V1 vs V2 schema parity.** The PR has two emitters but no test asserts
they produce equivalent JSON for an equivalent input. Add a parity test that
runs the same query under `useV1Command=true` and `useV1Command=false`, parses
both, and asserts equal `tables` arrays modulo ordering. This catches future
drift.
4. **Raw envelope assertion.** All assertions parse JSON back and check via
`\`. Add one literal assertion on the envelope to catch an accidental rename
`"tables" -> "results"`.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala:
##########
@@ -972,6 +986,45 @@ case class ShowTablesCommand(
Seq(Row(database, tableName, isTemp, s"$information\n"))
}
}
+
+ private def runAsJson(sparkSession: SparkSession): Seq[Row] = {
Review Comment:
`ShowTablesCommand.runAsJson` (V1) and `ShowTablesJsonExec` (V2)
are two emitters that have already drifted: extended catalog name is
`v2SessionCatalog.name()` here vs `catalog.name()` in V2; V1 uses
`SessionCatalog.listTables` (includes temp views) while V2 has a guarded
`listTempViews` branch.
**Suggestion:** collapse V1 + V2 into one `ShowTablesJsonCommand` built in
`AstBuilder.visitShowTables` / `visitShowTableExtended` after resolution,
pattern-matching on the resolved child (V1 session, V2 session, non-session
V2). Drop `asJson` from `ShowTables` / `ShowTablesExtended` /
`ShowTablesCommand` and the corresponding `stringArgs` overrides. The two emit
functions become one and the drift class closes.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesJsonExec.scala:
##########
@@ -0,0 +1,115 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.datasources.v2
+
+import scala.collection.mutable.ArrayBuffer
+
+import org.json4s._
+import org.json4s.jackson.JsonMethods._
+
+import org.apache.spark.sql.catalyst.InternalRow
+import org.apache.spark.sql.catalyst.expressions.Attribute
+import org.apache.spark.sql.catalyst.util.StringUtils
+import org.apache.spark.sql.connector.catalog.{CatalogV2Util, Identifier, TableCatalog, TableViewCatalog}
+import org.apache.spark.sql.execution.LeafExecNode
+
+/**
+ * Physical plan node for `SHOW TABLES AS JSON` and `SHOW TABLE EXTENDED AS JSON`.
+ *
+ * For a [[TableViewCatalog]] (non-extended only), listing is done via
+ * [[TableViewCatalog#listTableAndViewSummaries]] so that views appear alongside tables,
+ * matching the v1 `SHOW TABLES` semantics.
+ */
+case class ShowTablesJsonExec(
+ output: Seq[Attribute],
+ catalog: TableCatalog,
+ namespace: Seq[String],
+ pattern: String,
+ isExtended: Boolean) extends V2CommandExec with LeafExecNode {
+
+ override protected def run(): Seq[InternalRow] = {
+ val identifiers: Array[Identifier] = if (!isExtended) {
+ catalog match {
+ case mc: TableViewCatalog =>
+ mc.listTableAndViewSummaries(namespace.toArray).map(_.identifier())
+ case _ => catalog.listTables(namespace.toArray)
+ }
+ } else {
+ catalog.listTables(namespace.toArray)
+ }
+
+ val filteredIdents = identifiers.filter { ident =>
+ StringUtils.filterPattern(Seq(ident.name()), pattern).nonEmpty
+ }
+
+ val jsonRows = new ArrayBuffer[JObject]()
+ filteredIdents.foreach { ident =>
+ jsonRows += toJsonEntry(ident.name(), ident.namespace(), isTempView(ident))
+ }
+
+ // For non-session V2 catalogs that don't surface temp views via listTables() or
+ // listTableAndViewSummaries(), fetch them separately. For V2SessionCatalog,
+ // listTables() already includes local temp views, so we skip this to avoid duplicates.
+ // For TableViewCatalog (non-extended path), views come from listTableAndViewSummaries().
+ if (!CatalogV2Util.isSessionCatalog(catalog) &&
Review Comment:
This non-session V2 catalog branch
(`!CatalogV2Util.isSessionCatalog(catalog) && (isExtended ||
!catalog.isInstanceOf[TableViewCatalog])`) fetches session temp views from
`sparkSession.sessionState.catalog` and passes the V2 catalog's namespace as
the `db` argument. Local temp views live in the session catalog's reserved
temp database; global temp views live under `global_temp`. There is no
semantic in which a non-session V2 catalog's namespace contains session temp
views. Two potential problems:
1. The first match arm passes `namespace.head` as `db`. Passing a non-session
V2 catalog's namespace as the session catalog's temp DB is a category error.
2. The fallback arm assigns `db = ""` for multi-component namespaces.
`SessionCatalog.listTempViews` interprets `""` as local temp DB - silent
semantic mismatch.
**Suggestion:** Remove the branch
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "asl3 (via GitHub)" <gi...@apache.org>.
asl3 commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3151756255
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesExtendedExec.scala:
##########
@@ -73,6 +85,46 @@ case class ShowTablesExtendedExec(
rows.toSeq
}
+ private def runAsJson(): Seq[InternalRow] = {
+ val jsonRows = new ArrayBuffer[JObject]()
+
+ // fetch tables
+ val tables = catalog.listTables(namespace.toArray)
+ tables.foreach { tableIdent =>
+ if (StringUtils.filterPattern(Seq(tableIdent.name()), pattern).nonEmpty) {
+ // V2 persistent tables are always TABLE
+ jsonRows += JObject(
+ "catalog" -> JString(catalog.name()),
+ "namespace" -> JArray(tableIdent.namespace().map(JString(_)).toList),
+ "name" -> JString(tableIdent.name()),
+ "type" -> JString("TABLE"),
+ "isTemporary" -> JBool(false)
+ )
+ }
+ }
+
+ // fetch temp views, includes: global temp view, local temp view
+ val sessionCatalog = session.sessionState.catalog
+ val db = namespace match {
+ case Seq(db) => Some(db)
+ case _ => None
+ }
+ val tempViews = sessionCatalog.listTempViews(db.getOrElse(""), pattern)
Review Comment:
For V2SessionCatalog, `catalog.listTables(namespace.toArray)` already includes local temp views (and globals when ns is global_temp) via `SessionCatalog.listTables(db, "*", includeLocalTempViews=true)`.
This block lists them again, so each temp view is emitted twice: once with `type=TABLE, isTemporary=false`, once with `type=VIEW, isTemporary=true`. The same duplicate path was removed from ShowTablesExec.
Should either skip this `listTempViews` block (matching ShowTablesExec) and use an isTempView-style helper above, or filter temp views out of tables before emission.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowTablesSuiteBase.scala:
##########
@@ -461,4 +466,179 @@ trait ShowTablesSuiteBase extends QueryTest with DDLCommandTestUtils {
}
}
}
+
+ test("SHOW TABLES AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ val tblEntry = tables.find(t => (t \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON with empty database") {
+ withNamespace(s"$catalog.ns_empty") {
+ sql(s"CREATE NAMESPACE $catalog.ns_empty")
+ val df = sql(s"SHOW TABLES IN $catalog.ns_empty AS JSON")
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.isEmpty)
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1)
+ val entry = tables.head
+ assert((entry \ "name").extract[String] == "tbl")
+ assert((entry \ "type").extract[String] == "TABLE")
+ assert((entry \ "isTemporary").extract[Boolean] == false)
+ assert((entry \ "catalog").isInstanceOf[JString])
+ assert((entry \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON includes temp views") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ withTempView("tv") {
+ sql("CREATE TEMP VIEW tv AS SELECT 1 AS id")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ val tempView = tables.find(t => (t \ "name").extract[String] == "tv")
+ assert(tempView.isDefined)
+ assert((tempView.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with local temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val localTmpViewName = "tbl_local_tmp"
+ withTempView(localTmpViewName) {
+ sql(s"CREATE TEMPORARY VIEW $localTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl*' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ val tblEntry = tables.find(e => (e \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "type").extract[String] == "TABLE")
+
+ val tempViewEntry = tables.find(e => (e \ "name").extract[String] == localTmpViewName)
+ assert(tempViewEntry.isDefined)
+ assert((tempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ assert((tempViewEntry.get \ "type").extract[String] == "VIEW")
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with global temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val globalTmpViewName = "ext_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $globalNamespace LIKE 'ext_json*' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ val globalTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == globalTmpViewName)
+ assert(globalTempViewEntry.isDefined)
+ assert((globalTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ assert((globalTempViewEntry.get \ "type").extract[String] == "VIEW")
+ }
+ }
+ }
+
+ test("SHOW TABLES AS JSON with global temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val globalTmpViewName = "show_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLES IN $globalNamespace AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ val globalTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == globalTmpViewName)
+ assert(globalTempViewEntry.isDefined)
+ assert((globalTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with both local and global temp views") {
Review Comment:
seems @cloud-fan 's comment about .find() masking duplicates still needs to be addressed here
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesExtendedExec.scala:
##########
@@ -73,6 +85,46 @@ case class ShowTablesExtendedExec(
rows.toSeq
}
+ private def runAsJson(): Seq[InternalRow] = {
+ val jsonRows = new ArrayBuffer[JObject]()
+
+ // fetch tables
+ val tables = catalog.listTables(namespace.toArray)
+ tables.foreach { tableIdent =>
+ if (StringUtils.filterPattern(Seq(tableIdent.name()), pattern).nonEmpty) {
+ // V2 persistent tables are always TABLE
Review Comment:
The comment "V2 persistent tables are always TABLE" and unconditional type=TABLE, isTemporary=false is wrong when catalog is V2SessionCatalog. Can use a per-row `isTempView` check (example: ShowTablesExec.isTempView)
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala:
##########
@@ -970,6 +982,48 @@ case class ShowTablesCommand(
Seq(Row(database, tableName, isTemp, s"$information\n"))
}
}
+
+ private def runAsJson(sparkSession: SparkSession): Seq[Row] = {
+ val catalog = sparkSession.sessionState.catalog
+ val db = databaseName.getOrElse(catalog.getCurrentDatabase)
+ val tables =
+ tableIdentifierPattern.map(catalog.listTables(db, _)).getOrElse(catalog.listTables(db))
+
+ val jsonTables = tables.map { tableIdent =>
+ val isTemp = catalog.isTempView(tableIdent)
+ val ns = tableIdent.database.toList
+
+ if (isExtended) {
+ val tableType = if (isTemp) {
+ "VIEW"
+ } else {
+ Try(catalog.getTempViewOrPermanentTableMetadata(tableIdent)) match {
+ case Success(meta) if meta.tableType == CatalogTableType.VIEW => "VIEW"
+ case Success(_) => "TABLE"
+ case Failure(_) => "UNKNOWN"
Review Comment:
Failure(_) => "UNKNOWN" swallows any exception from `getTempViewOrPermanentTableMetadata`. The text path lets exceptions propagate - consider following the same here
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala:
##########
@@ -1313,6 +1313,7 @@ case class RenameTable(
case class ShowTables(
namespace: LogicalPlan,
pattern: Option[String],
+ asJson: Boolean = false,
Review Comment:
Currently have duplicated JSON emission code across Exec classes. Consider a ShowTablesJsonCommand which takes the resolved namespace/catalog and handles all variants centrally, similar to DescribeRelationJsonCommand
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowTablesSuiteBase.scala:
##########
@@ -461,4 +466,179 @@ trait ShowTablesSuiteBase extends QueryTest with DDLCommandTestUtils {
}
}
}
+
+ test("SHOW TABLES AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ val tblEntry = tables.find(t => (t \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON with empty database") {
+ withNamespace(s"$catalog.ns_empty") {
+ sql(s"CREATE NAMESPACE $catalog.ns_empty")
+ val df = sql(s"SHOW TABLES IN $catalog.ns_empty AS JSON")
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.isEmpty)
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON returns single row with json_metadata column") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT, data STRING) $defaultUsing")
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ assert(tables.length == 1)
+ val entry = tables.head
+ assert((entry \ "name").extract[String] == "tbl")
+ assert((entry \ "type").extract[String] == "TABLE")
+ assert((entry \ "isTemporary").extract[Boolean] == false)
+ assert((entry \ "catalog").isInstanceOf[JString])
+ assert((entry \ "namespace").isInstanceOf[JArray])
+ }
+ }
+
+ test("SHOW TABLES AS JSON includes temp views") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ withTempView("tv") {
+ sql("CREATE TEMP VIEW tv AS SELECT 1 AS id")
+ val df = sql(s"SHOW TABLES IN $catalog.ns AS JSON")
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+ val tempView = tables.find(t => (t \ "name").extract[String] == "tv")
+ assert(tempView.isDefined)
+ assert((tempView.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with local temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val localTmpViewName = "tbl_local_tmp"
+ withTempView(localTmpViewName) {
+ sql(s"CREATE TEMPORARY VIEW $localTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $catalog.ns LIKE 'tbl*' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ val tblEntry = tables.find(e => (e \ "name").extract[String] == "tbl")
+ assert(tblEntry.isDefined)
+ assert((tblEntry.get \ "isTemporary").extract[Boolean] == false)
+ assert((tblEntry.get \ "type").extract[String] == "TABLE")
+
+ val tempViewEntry = tables.find(e => (e \ "name").extract[String] == localTmpViewName)
+ assert(tempViewEntry.isDefined)
+ assert((tempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ assert((tempViewEntry.get \ "type").extract[String] == "VIEW")
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with global temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val globalTmpViewName = "ext_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $globalNamespace LIKE 'ext_json*' AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ val globalTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == globalTmpViewName)
+ assert(globalTempViewEntry.isDefined)
+ assert((globalTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ assert((globalTempViewEntry.get \ "type").extract[String] == "VIEW")
+ }
+ }
+ }
+
+ test("SHOW TABLES AS JSON with global temp view") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val globalTmpViewName = "show_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLES IN $globalNamespace AS JSON")
+ assert(df.schema.fieldNames === Seq("json_metadata"))
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ val globalTempViewEntry =
+ tables.find(e => (e \ "name").extract[String] == globalTmpViewName)
+ assert(globalTempViewEntry.isDefined)
+ assert((globalTempViewEntry.get \ "isTemporary").extract[Boolean] == true)
+ }
+ }
+ }
+
+ test("SHOW TABLE EXTENDED AS JSON with both local and global temp views") {
+ withNamespaceAndTable("ns", "tbl") { t =>
+ sql(s"CREATE TABLE $t (id INT) $defaultUsing")
+ val localTmpViewName = "both_json_ltv"
+ val globalTmpViewName = "both_json_gtv"
+ val globalNamespace = "global_temp"
+ withView(localTmpViewName, s"$globalNamespace.$globalTmpViewName") {
+ sql(s"CREATE OR REPLACE TEMP VIEW $localTmpViewName AS SELECT id FROM $t")
+ sql(s"CREATE OR REPLACE GLOBAL TEMP VIEW $globalTmpViewName AS SELECT id FROM $t")
+
+ val df = sql(s"SHOW TABLE EXTENDED IN $globalNamespace LIKE 'both_json*' AS JSON")
+ assert(df.count() == 1)
+
+ val jsonStr = df.collect()(0).getString(0)
+ val json = parse(jsonStr)
+ val tables = (json \ "tables").asInstanceOf[JArray].arr
+
+ assert(tables.length == 2)
Review Comment:
this test seems failing in CI?
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesExec.scala:
##########
@@ -33,18 +36,32 @@ case class ShowTablesExec(
output: Seq[Attribute],
catalog: TableCatalog,
namespace: Seq[String],
- pattern: Option[String]) extends V2CommandExec with LeafExecNode {
+ pattern: Option[String],
+ asJson: Boolean = false) extends V2CommandExec with LeafExecNode {
override protected def run(): Seq[InternalRow] = {
- val rows = new ArrayBuffer[InternalRow]()
-
val tables = catalog.listTables(namespace.toArray)
- tables.map { table =>
- if (pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)) {
+ val filteredTables = tables.filter { table =>
+ pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)
+ }
+
+ if (asJson) {
+ val jsonTables = filteredTables.map { table =>
+ JObject(
+ "name" -> JString(table.name()),
Review Comment:
field order should be consistent between non-extended `{name, namespace, isTemporary}` and extended `{catalog, namespace, name, type, isTemporary}`
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3208026323
##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -7418,6 +7418,11 @@
"DESC TABLE COLUMN for a specific partition."
]
},
+ "SHOW_TABLE_EXTENDED_JSON_WITH_PARTITION" : {
Review Comment:
Fixed - SHOW_TABLE_EXTENDED_JSON_WITH_PARTITION is now placed between SET_VARIABLE_USING_SET and SQL_CURSOR to maintain alphabetical order.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3208530194
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala:
##########
@@ -938,6 +950,48 @@ case class ShowTablesCommand(
Seq(Row(database, tableName, isTemp, s"$information\n"))
}
}
+
+ private def runAsJson(sparkSession: SparkSession): Seq[Row] = {
+ val catalog = sparkSession.sessionState.catalog
+ val db = databaseName.getOrElse(catalog.getCurrentDatabase)
+ val tables =
+ tableIdentifierPattern.map(catalog.listTables(db, _)).getOrElse(catalog.listTables(db))
+
+ val jsonTables = tables.map { tableIdent =>
+ val isTemp = catalog.isTempView(tableIdent)
+ val ns = tableIdent.database.toList
+
+ if (isExtended) {
+ val tableType = if (isTemp) {
+ "VIEW"
+ } else {
+ Try(catalog.getTempViewOrPermanentTableMetadata(tableIdent)) match {
+ case Success(meta) if meta.tableType == CatalogTableType.VIEW => "VIEW"
+ case Success(_) => "TABLE"
+ case Failure(_) => "UNKNOWN"
+ }
+ }
+
+ JObject(
+ "catalog" -> JString(
+ sparkSession.sessionState.catalogManager.currentCatalog.name()),
Review Comment:
Fixed - now uses sparkSession.sessionState.catalogManager.v2SessionCatalog.name() as suggested. This correctly returns spark_catalog regardless of which catalog the user has set active via USE.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3021069038
##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -7418,6 +7418,11 @@
"DESC TABLE COLUMN for a specific partition."
]
},
+ "SHOW_TABLE_EXTENDED_JSON_WITH_PARTITION" : {
Review Comment:
The `UNSUPPORTED_FEATURE` sub-classes are alphabetically ordered. `SHOW_TABLE_EXTENDED_JSON_WITH_PARTITION` (S-H) is placed between `DESC_TABLE_COLUMN_PARTITION` (D-E) and `DROP_DATABASE` (D-R), but it should be placed between `SET_VARIABLE_USING_SET` and `SQL_CURSOR` to maintain alphabetical order.
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala:
##########
@@ -938,6 +950,48 @@ case class ShowTablesCommand(
Seq(Row(database, tableName, isTemp, s"$information\n"))
}
}
+
+ private def runAsJson(sparkSession: SparkSession): Seq[Row] = {
+ val catalog = sparkSession.sessionState.catalog
+ val db = databaseName.getOrElse(catalog.getCurrentDatabase)
+ val tables =
+ tableIdentifierPattern.map(catalog.listTables(db, _)).getOrElse(catalog.listTables(db))
+
+ val jsonTables = tables.map { tableIdent =>
+ val isTemp = catalog.isTempView(tableIdent)
+ val ns = tableIdent.database.toList
+
+ if (isExtended) {
+ val tableType = if (isTemp) {
+ "VIEW"
+ } else {
+ Try(catalog.getTempViewOrPermanentTableMetadata(tableIdent)) match {
+ case Success(meta) if meta.tableType == CatalogTableType.VIEW => "VIEW"
+ case Success(_) => "TABLE"
+ case Failure(_) => "UNKNOWN"
+ }
+ }
+
+ JObject(
+ "catalog" -> JString(
+ sparkSession.sessionState.catalogManager.currentCatalog.name()),
Review Comment:
`currentCatalog` returns the user's currently active catalog (set via `USE`), but `ShowTablesCommand` always operates on the session catalog (matched via `ResolvedV1Database`). If the user runs e.g. `USE my_v2_catalog; SHOW TABLE EXTENDED IN spark_catalog.mydb LIKE '*' AS JSON`, this reports `"catalog": "my_v2_catalog"` instead of `"spark_catalog"`. The V2 exec correctly uses `catalog.name()` instead.
```suggestion
"catalog" -> JString(
sparkSession.sessionState.catalogManager.v2SessionCatalog.name()),
```
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesExec.scala:
##########
@@ -33,18 +36,51 @@ case class ShowTablesExec(
output: Seq[Attribute],
catalog: TableCatalog,
namespace: Seq[String],
- pattern: Option[String]) extends V2CommandExec with LeafExecNode {
+ pattern: Option[String],
+ asJson: Boolean = false) extends V2CommandExec with LeafExecNode {
override protected def run(): Seq[InternalRow] = {
- val rows = new ArrayBuffer[InternalRow]()
-
val tables = catalog.listTables(namespace.toArray)
- tables.map { table =>
- if (pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)) {
+ val filteredTables = tables.filter { table =>
+ pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)
+ }
+
+ if (asJson) {
+ val jsonTables = filteredTables.map { table =>
+ JObject(
+ "name" -> JString(table.name()),
+ "namespace" -> JArray(table.namespace().map(JString(_)).toList),
+ "isTemporary" -> JBool(isTempView(table, catalog))
+ )
+ }.toList
+
+ val jsonTempViews = if (CatalogV2Util.isSessionCatalog(catalog)) {
+ val sessionCatalog = session.sessionState.catalog
Review Comment:
`catalog.listTables()` via `V2SessionCatalog` calls `SessionCatalog.listTables(db, "*", includeLocalTempViews=true)` which already includes local temp views. This separate `listTempViews()` call adds them a second time, causing duplicate entries in the JSON output when the V2 session catalog is used (when `useV1Command=false`).
The non-JSON path at line 78 does not list temp views separately — it correctly relies on what `catalog.listTables()` returns.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3335832573
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala:
##########
@@ -1371,17 +1371,22 @@ case class RenameTable(
case class ShowTables(
namespace: LogicalPlan,
pattern: Option[String],
+ asJson: Boolean = false,
override val output: Seq[Attribute] = ShowTables.getOutputAttrs) extends UnaryCommand {
override def child: LogicalPlan = namespace
override protected def withNewChildInternal(newChild: LogicalPlan): ShowTables =
copy(namespace = newChild)
+ override protected def stringArgs: Iterator[Any] = Iterator(pattern, output)
Review Comment:
`stringArgs` override was added as a workaround for golden-file CI failures caused by adding `asJson: Boolean = false` to ShowTables/ShowTablesExtended/ShowTablesCommand (changing their simpleString output).
We plan to fix this by removing asJson from those plan nodes entirely. Since `AS JSON` queries will create a dedicated command node, there will be no reason for `ShowTables`/`ShowTablesExtended`/`ShowTablesCommand` to carry `asJson` at all making the stringArgs workaround unnecessary.
Does this approach match your intent?
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3107285780
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesExec.scala:
##########
@@ -33,18 +36,51 @@ case class ShowTablesExec(
output: Seq[Attribute],
catalog: TableCatalog,
namespace: Seq[String],
- pattern: Option[String]) extends V2CommandExec with LeafExecNode {
+ pattern: Option[String],
+ asJson: Boolean = false) extends V2CommandExec with LeafExecNode {
override protected def run(): Seq[InternalRow] = {
- val rows = new ArrayBuffer[InternalRow]()
-
val tables = catalog.listTables(namespace.toArray)
- tables.map { table =>
- if (pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)) {
+ val filteredTables = tables.filter { table =>
+ pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)
+ }
+
+ if (asJson) {
+ val jsonTables = filteredTables.map { table =>
+ JObject(
+ "name" -> JString(table.name()),
+ "namespace" -> JArray(table.namespace().map(JString(_)).toList),
+ "isTemporary" -> JBool(isTempView(table, catalog))
+ )
+ }.toList
+
+ val jsonTempViews = if (CatalogV2Util.isSessionCatalog(catalog)) {
+ val sessionCatalog = session.sessionState.catalog
Review Comment:
Good catch! I will remove the redundant sessionCatalog.listTempViews(...) block entirely.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3107287151
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala:
##########
@@ -938,6 +950,48 @@ case class ShowTablesCommand(
Seq(Row(database, tableName, isTemp, s"$information\n"))
}
}
+
+ private def runAsJson(sparkSession: SparkSession): Seq[Row] = {
+ val catalog = sparkSession.sessionState.catalog
+ val db = databaseName.getOrElse(catalog.getCurrentDatabase)
+ val tables =
+ tableIdentifierPattern.map(catalog.listTables(db, _)).getOrElse(catalog.listTables(db))
+
+ val jsonTables = tables.map { tableIdent =>
+ val isTemp = catalog.isTempView(tableIdent)
+ val ns = tableIdent.database.toList
+
+ if (isExtended) {
+ val tableType = if (isTemp) {
+ "VIEW"
+ } else {
+ Try(catalog.getTempViewOrPermanentTableMetadata(tableIdent)) match {
+ case Success(meta) if meta.tableType == CatalogTableType.VIEW => "VIEW"
+ case Success(_) => "TABLE"
+ case Failure(_) => "UNKNOWN"
+ }
+ }
+
+ JObject(
+ "catalog" -> JString(
+ sparkSession.sessionState.catalogManager.currentCatalog.name()),
Review Comment:
Thanks for pointing out the scoping distinction with USE. I'll update the implementation.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on PR #54824:
URL: https://github.com/apache/spark/pull/54824#issuecomment-4151073721
@asl3 Have you had the opportunity to review this pull request yet?
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3362670138
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/command/ShowTablesJsonCommand.scala:
##########
@@ -0,0 +1,152 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command
+
+import scala.collection.mutable.ArrayBuffer
+
+import org.json4s._
+import org.json4s.jackson.JsonMethods._
+
+import org.apache.spark.sql.{Row, SparkSession}
+import org.apache.spark.sql.catalyst.analysis.ResolvedNamespace
+import org.apache.spark.sql.catalyst.catalog.CatalogTableType
+import org.apache.spark.sql.catalyst.expressions.{Attribute, AttributeReference}
+import org.apache.spark.sql.catalyst.plans.logical.LogicalPlan
+import org.apache.spark.sql.catalyst.util.StringUtils
+import org.apache.spark.sql.connector.catalog.{CatalogV2Util, TableCatalog, TableViewCatalog}
+import org.apache.spark.sql.connector.catalog.CatalogV2Implicits._
+import org.apache.spark.sql.types.{MetadataBuilder, StringType}
+
+/**
+ * The command for `SHOW TABLES AS JSON` and `SHOW TABLE EXTENDED AS JSON`.
+ */
+case class ShowTablesJsonCommand(
+ child: LogicalPlan,
+ pattern: Option[String],
+ isExtended: Boolean,
+ override val output: Seq[Attribute] = Seq(
+ AttributeReference(
+ "json_metadata",
+ StringType,
+ nullable = false,
+ new MetadataBuilder()
+ .putString("comment", "JSON metadata of the tables")
+ .build())()
+ )) extends UnaryRunnableCommand {
+
+ override def run(sparkSession: SparkSession): Seq[Row] = {
+ val jsonOutput = child match {
+ case ResolvedNamespace(catalog, ns, _) =>
+ if (CatalogV2Util.isSessionCatalog(catalog)) {
+ runForSessionCatalog(sparkSession, ns)
+ } else {
+ runForV2Catalog(sparkSession, catalog.asTableCatalog, ns)
+ }
+ }
+ Seq(Row(compact(render(jsonOutput))))
+ }
+
+ private def runForSessionCatalog(
+ sparkSession: SparkSession,
+ ns: Seq[String]): JObject = {
+ val sessionCatalog = sparkSession.sessionState.catalog
+ val db = ns.headOption.getOrElse(sessionCatalog.getCurrentDatabase)
+ val tables = pattern
+ .map(p => sessionCatalog.listTables(db, p))
+ .getOrElse(sessionCatalog.listTables(db))
+
+ val jsonTables = tables.map { tableIdent =>
+ val isTemp = sessionCatalog.isTempView(tableIdent)
+ val namespace = tableIdent.database.toList
+
+ if (isExtended) {
+ val tableType = if (isTemp) {
+ "VIEW"
+ } else {
+ val meta = sessionCatalog.getTempViewOrPermanentTableMetadata(tableIdent)
+ if (meta.tableType == CatalogTableType.VIEW) "VIEW" else "TABLE"
Review Comment:
Good catch. The coarse TABLE/VIEW was carried over from the old `ShowTablesJsonExec` and is not an intentional design choice. Will switch to `meta.tableType.name `to align with the text `SHOW TABLE EXTENDED `output and avoid the `METRIC_VIEW` bug.
For the V2 path (line 135), will keep TABLE since we'd need a per-table loadTable to resolve the actual type.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3208571236
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesExtendedExec.scala:
##########
@@ -73,6 +85,46 @@ case class ShowTablesExtendedExec(
rows.toSeq
}
+ private def runAsJson(): Seq[InternalRow] = {
+ val jsonRows = new ArrayBuffer[JObject]()
+
+ // fetch tables
+ val tables = catalog.listTables(namespace.toArray)
+ tables.foreach { tableIdent =>
+ if (StringUtils.filterPattern(Seq(tableIdent.name()), pattern).nonEmpty) {
+ // V2 persistent tables are always TABLE
Review Comment:
Fixed. In ShowTablesJsonExec, each identifier from listTables() / listTableAndViewSummaries() is now passed through isTempView(ident), which checks SessionCatalog.isTempView when catalog is V2SessionCatalog.
Temp views returned by listTables() are correctly identified and emitted with type=VIEW, isTemporary=true.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3208588525
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/v2Commands.scala:
##########
@@ -1313,6 +1313,7 @@ case class RenameTable(
case class ShowTables(
namespace: LogicalPlan,
pattern: Option[String],
+ asJson: Boolean = false,
Review Comment:
Addressed. We extracted a single ShowTablesJsonExec physical plan node covering both SHOW TABLES AS JSON and SHOW TABLE EXTENDED AS JSON, controlled by an isExtended: Boolean flag.
--
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-53840][SQL] Add AS JSON output support for SHOW TABLES and SHOW TABLE EXTENDED [spark]
Posted by "ayushbilala (via GitHub)" <gi...@apache.org>.
ayushbilala commented on code in PR #54824:
URL: https://github.com/apache/spark/pull/54824#discussion_r3208523185
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/ShowTablesExec.scala:
##########
@@ -33,18 +36,51 @@ case class ShowTablesExec(
output: Seq[Attribute],
catalog: TableCatalog,
namespace: Seq[String],
- pattern: Option[String]) extends V2CommandExec with LeafExecNode {
+ pattern: Option[String],
+ asJson: Boolean = false) extends V2CommandExec with LeafExecNode {
override protected def run(): Seq[InternalRow] = {
- val rows = new ArrayBuffer[InternalRow]()
-
val tables = catalog.listTables(namespace.toArray)
- tables.map { table =>
- if (pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)) {
+ val filteredTables = tables.filter { table =>
+ pattern.map(StringUtils.filterPattern(Seq(table.name()), _).nonEmpty).getOrElse(true)
+ }
+
+ if (asJson) {
+ val jsonTables = filteredTables.map { table =>
+ JObject(
+ "name" -> JString(table.name()),
+ "namespace" -> JArray(table.namespace().map(JString(_)).toList),
+ "isTemporary" -> JBool(isTempView(table, catalog))
+ )
+ }.toList
+
+ val jsonTempViews = if (CatalogV2Util.isSessionCatalog(catalog)) {
+ val sessionCatalog = session.sessionState.catalog
Review Comment:
Fixed. JSON path has been moved to the new ShowTablesJsonExec which guards the separate listTempViews() call with !CatalogV2Util.isSessionCatalog(catalog), matching the logic of the non-JSON path.
When catalog is V2SessionCatalog, temp views are already included in listTables() results and the separate call is skipped.
--
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