HyukjinKwon commented on code in PR #59235: URL: https://github.com/apache/spark/pull/59235#discussion_r4221507698
########## sql/core/src/main/scala/org/apache/spark/sql/execution/command/ShowViewsJsonCommand.scala: ########## @@ -0,0 +1,117 @@ +/* + * 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 org.json4s._ +import org.json4s.jackson.JsonMethods.{compact, render} + +import org.apache.spark.SparkException +import org.apache.spark.sql.{Row, SparkSession} +import org.apache.spark.sql.catalyst.analysis.ResolvedNamespace +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, ViewCatalog} +import org.apache.spark.sql.errors.QueryCompilationErrors +import org.apache.spark.sql.types.{MetadataBuilder, StringType} + +/** + * The command for `SHOW VIEWS AS JSON`. + * + * The output is a single row with a `json_metadata` column holding a JSON object of the form: + * {{{ + * {"views":[{"namespace":["db"],"viewName":"v1","isTemporary":false}]} + * }}} + * + * The syntax of using this command in SQL is: + * {{{ + * SHOW VIEWS [(FROM | IN) namespace] [[LIKE] pattern] AS JSON; + * }}} + */ +case class ShowViewsJsonCommand( + child: LogicalPlan, + pattern: Option[String], + override val output: Seq[Attribute] = Seq( + AttributeReference( + "json_metadata", + StringType, + nullable = false, + new MetadataBuilder().putString("comment", "JSON metadata of the views").build())())) + extends UnaryRunnableCommand { + + override def run(sparkSession: SparkSession): Seq[Row] = { + val views = child match { + // A catalog that implements `ViewCatalog` owns its views, even when it overrides the + // session catalog. This mirrors the routing between `ResolveSessionCatalog` and + // `DataSourceV2Strategy` for the non-JSON `SHOW VIEWS`. + case ResolvedNamespace(catalog: ViewCatalog, ns, _) => + listViewCatalogViews(catalog, ns) + case ResolvedNamespace(catalog, ns, _) if CatalogV2Util.isSessionCatalog(catalog) => + listSessionCatalogViews(sparkSession, ns) + case ResolvedNamespace(catalog, _, _) => + throw QueryCompilationErrors.missingCatalogViewsAbilityError(catalog) + case other => + throw SparkException.internalError( + s"Unexpected child in ShowViewsJsonCommand: ${other.getClass.getSimpleName}") + } + Seq(Row(compact(render(JObject("views" -> JArray(views.toList)))))) + } + + private def listSessionCatalogViews( + sparkSession: SparkSession, + ns: Seq[String]): Seq[JObject] = { + val catalog = sparkSession.sessionState.catalog + val db = databaseInSessionCatalog(ns) + catalog.listViews(db, pattern.getOrElse("*")).map { ident => + toJson(ident.database.toSeq, ident.table, catalog.isTempView(ident)) + } + } + + private def listViewCatalogViews(catalog: ViewCatalog, ns: Seq[String]): Seq[JObject] = { + catalog.listViews(ns.toArray).filter { ident => + pattern.forall(p => StringUtils.filterPattern(Seq(ident.name), p).nonEmpty) + }.map { ident => + // v2 catalogs have no temp views, matching `ShowViewsExec`. + toJson(ident.namespace().toSeq, ident.name(), isTemporary = false) + }.toSeq + } + + private def toJson(namespace: Seq[String], viewName: String, isTemporary: Boolean): JObject = { + JObject( + "namespace" -> JArray(namespace.map(JString(_)).toList), + "viewName" -> JString(viewName), + "isTemporary" -> JBool(isTemporary)) + } + + /** + * Resolves the single-part database name the v1 session catalog requires, rejecting the same + * namespaces `ResolveSessionCatalog` rejects for the non-JSON `SHOW VIEWS`. `CurrentNamespace` + * always yields at least the current database, so the empty case below does not fire through Review Comment: **Nit (P3):** This branch is reachable through the normal parser path. `SHOW VIEWS IN spark_catalog AS JSON` resolves to `ResolvedNamespace(spark_catalog, Seq())` (via `CatalogAndNamespace` in `ResolveCatalogs`), so it lands in `case Seq()` and raises `MISSING_DATABASE_FOR_V1_SESSION_CATALOG`. That is the same error the tabular command gets from `ResolvedDatabaseInSessionCatalog`. The behavior is right today, but this comment invites deleting the arm as dead code. If someone did, this input would fall to `case _` and report a nested-namespace error with an empty namespace. Could you reword the comment to name this input, and add `SHOW VIEWS IN $catalog`, in both the plain and `AS JSON` forms, to the v1 error-parity tests? **Verification:** - **Regression:** A v1 SHOW VIEWS test asserts that the catalog-only namespace fails with MISSING_DATABASE_FOR_V1_SESSION_CATALOG for both the plain and AS JSON forms, so removing the empty-namespace arm would fail it. - **Inspection:** The databaseInSessionCatalog scaladoc accurately describes which inputs reach the empty-namespace case. ########## sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowViewsSuiteBase.scala: ########## @@ -64,4 +79,69 @@ trait ShowViewsSuiteBase extends QueryTest with DDLCommandTestUtils { s"isTemporary must be false for persistent view rows: $r")) } } + + test("AS JSON returns a single json_metadata column") { + sql(s"CREATE VIEW $catalog.$namespace.v_json_schema AS SELECT 1 AS x") + val df = sql(s"SHOW VIEWS IN $catalog.$namespace AS JSON") + assert(df.schema.length == 1) + assert(df.schema.head.name == "json_metadata") + } + + test("AS JSON lists user-created views") { + sql(s"CREATE VIEW $catalog.$namespace.v_json_a AS SELECT 1 AS x") + sql(s"CREATE VIEW $catalog.$namespace.v_json_b AS SELECT 2 AS x") + val views = showViewsAsJson(s"SHOW VIEWS IN $catalog.$namespace AS JSON") + val names = viewNames(views) + assert(names.contains("v_json_a"), s"v_json_a missing: $names") + assert(names.contains("v_json_b"), s"v_json_b missing: $names") + assert(names.distinct.length == names.length, s"duplicate entries: $names") + // Scope to the two views this test created: this suite also runs under v1, where a + // leftover temp view (empty namespace, isTemporary=true) would otherwise fail the asserts. + val created = views.filter(v => names.contains((v \ "viewName").extract[String])) Review Comment: **Nit (P3):** `names` is built from `views` itself, so `names.contains(...)` is true for every entry and this filter keeps everything. The asserts below still run on any leftover local temp view, which `SessionCatalog.listViews` appends to every database listing. That is exactly the case the comment says this filter guards against. Filtering on the two names this test created, for example `Set("v_json_a", "v_json_b").contains(...)`, would make the scoping real. **Verification:** - **Behavior:** The AS JSON listing test still passes under both the v1 and v2 suites and still asserts namespace == [namespace] and isTemporary == false for exactly the two created views. ########## sql/core/src/test/scala/org/apache/spark/sql/execution/command/v1/ShowViewsSuite.scala: ########## @@ -17,6 +17,83 @@ package org.apache.spark.sql.execution.command.v1 +import org.apache.spark.sql.AnalysisException import org.apache.spark.sql.execution.command -class ShowViewsSuite extends command.ShowViewsSuiteBase with ViewCommandSuiteBase +/** + * Temp views live in the session catalog only, so they are covered here rather than in the + * unified `ShowViewsSuiteBase`. + */ +class ShowViewsSuite extends command.ShowViewsSuiteBase with ViewCommandSuiteBase { + + test("AS JSON reports a local temp view as temporary") { + withTempView("v_json_local_temp") { + sql("CREATE TEMPORARY VIEW v_json_local_temp AS SELECT 1 AS x") + val views = showViewsAsJson(s"SHOW VIEWS IN $catalog.$namespace AS JSON") + val temp = views.filter(v => (v \ "viewName").extract[String] == "v_json_local_temp") + assert(temp.length == 1, s"expected exactly one entry: ${viewNames(views)}") + assert((temp.head \ "isTemporary").extract[Boolean]) + // A local temp view belongs to no database, so its namespace is empty. + assert((temp.head \ "namespace").extract[Seq[String]].isEmpty) + } + } + + test("AS JSON lists global temp views under the global temp database") { + withGlobalTempView("v_json_global_temp") { + sql("CREATE GLOBAL TEMPORARY VIEW v_json_global_temp AS SELECT 1 AS x") + val globalTempDb = spark.sharedState.globalTempDB + val views = showViewsAsJson(s"SHOW VIEWS IN $globalTempDb AS JSON") + val temp = views.filter(v => (v \ "viewName").extract[String] == "v_json_global_temp") + assert(temp.length == 1, s"expected exactly one entry: ${viewNames(views)}") + assert((temp.head \ "isTemporary").extract[Boolean]) + assert((temp.head \ "namespace").extract[Seq[String]] == Seq(globalTempDb)) + } + } + + test("AS JSON agrees with the tabular output for a global temp view") { + withGlobalTempView("v_json_global_parity") { + sql("CREATE GLOBAL TEMPORARY VIEW v_json_global_parity AS SELECT 1 AS x") + val globalTempDb = spark.sharedState.globalTempDB + val expected = sql(s"SHOW VIEWS IN $globalTempDb").collect() + .map(r => (r.getString(0), r.getString(1), r.getBoolean(2))).toSet + val actual = showViewsAsJson(s"SHOW VIEWS IN $globalTempDb AS JSON").map { v => + ((v \ "namespace").extract[Seq[String]].mkString("."), + (v \ "viewName").extract[String], + (v \ "isTemporary").extract[Boolean]) + }.toSet + assert(actual == expected) + } + } + + test("AS JSON without IN or FROM uses the current namespace") { + sql(s"USE $catalog.$namespace") Review Comment: **Nit (P3):** `spark_catalog.default` is already the session's starting namespace, and it is where every other test creates its views. So this test would still pass if `SHOW VIEWS AS JSON` ignored the current namespace and always listed `default`. Could you create a separate database, `USE` it, create the view there, and also assert that a view that exists only in `default` is not listed? Please restore the current namespace afterwards. **Verification:** - **Regression:** After USE of a non-default database, SHOW VIEWS AS JSON lists that database's view and excludes a view present only in default. ########## sql/core/src/main/scala/org/apache/spark/sql/execution/command/ShowViewsJsonCommand.scala: ########## @@ -0,0 +1,117 @@ +/* + * 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 org.json4s._ +import org.json4s.jackson.JsonMethods.{compact, render} + +import org.apache.spark.SparkException +import org.apache.spark.sql.{Row, SparkSession} +import org.apache.spark.sql.catalyst.analysis.ResolvedNamespace +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, ViewCatalog} +import org.apache.spark.sql.errors.QueryCompilationErrors +import org.apache.spark.sql.types.{MetadataBuilder, StringType} + +/** + * The command for `SHOW VIEWS AS JSON`. + * + * The output is a single row with a `json_metadata` column holding a JSON object of the form: + * {{{ + * {"views":[{"namespace":["db"],"viewName":"v1","isTemporary":false}]} + * }}} + * + * The syntax of using this command in SQL is: + * {{{ + * SHOW VIEWS [(FROM | IN) namespace] [[LIKE] pattern] AS JSON; + * }}} + */ +case class ShowViewsJsonCommand( + child: LogicalPlan, + pattern: Option[String], + override val output: Seq[Attribute] = Seq( + AttributeReference( + "json_metadata", + StringType, + nullable = false, + new MetadataBuilder().putString("comment", "JSON metadata of the views").build())())) + extends UnaryRunnableCommand { + + override def run(sparkSession: SparkSession): Seq[Row] = { + val views = child match { + // A catalog that implements `ViewCatalog` owns its views, even when it overrides the + // session catalog. This mirrors the routing between `ResolveSessionCatalog` and + // `DataSourceV2Strategy` for the non-JSON `SHOW VIEWS`. + case ResolvedNamespace(catalog: ViewCatalog, ns, _) => + listViewCatalogViews(catalog, ns) + case ResolvedNamespace(catalog, ns, _) if CatalogV2Util.isSessionCatalog(catalog) => + listSessionCatalogViews(sparkSession, ns) + case ResolvedNamespace(catalog, _, _) => + throw QueryCompilationErrors.missingCatalogViewsAbilityError(catalog) + case other => + throw SparkException.internalError( + s"Unexpected child in ShowViewsJsonCommand: ${other.getClass.getSimpleName}") + } + Seq(Row(compact(render(JObject("views" -> JArray(views.toList)))))) + } + + private def listSessionCatalogViews( + sparkSession: SparkSession, + ns: Seq[String]): Seq[JObject] = { + val catalog = sparkSession.sessionState.catalog + val db = databaseInSessionCatalog(ns) + catalog.listViews(db, pattern.getOrElse("*")).map { ident => + toJson(ident.database.toSeq, ident.table, catalog.isTempView(ident)) + } + } + + private def listViewCatalogViews(catalog: ViewCatalog, ns: Seq[String]): Seq[JObject] = { + catalog.listViews(ns.toArray).filter { ident => + pattern.forall(p => StringUtils.filterPattern(Seq(ident.name), p).nonEmpty) + }.map { ident => + // v2 catalogs have no temp views, matching `ShowViewsExec`. + toJson(ident.namespace().toSeq, ident.name(), isTemporary = false) Review Comment: **Nit (P3):** Emitting `namespace` as an array of parts is the main structural difference from the tabular output, but every execution test uses the single-part `default`. Collapsing `ns1.ns2` into one element, or keeping only the first part, would not fail any test. `InMemoryRelationCatalog` (the v2 suite's `test_view_catalog`) supports nested namespaces. A v2 test that creates a view under `test_view_catalog.ns1.ns2` and asserts `"namespace":["ns1","ns2"]` would cover this. **Verification:** - **Regression:** A nested-namespace v2 listing asserts the namespace array element by element, so collapsing or truncating the parts fails the test. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
