spark-test commented on code in PR #59235:
URL: https://github.com/apache/spark/pull/59235#discussion_r4394927386
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowViewsSuiteBase.scala:
##########
@@ -64,4 +79,72 @@ 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.
+ // Uses a literal set, not `names` (which is derived from `views` itself
and would match
+ // everything, silently disabling this scoping).
+ val createdNames = Set("v_json_a", "v_json_b")
+ val created = views.filter(v => createdNames.contains((v \
"viewName").extract[String]))
+ created.foreach { v =>
+ assert((v \ "namespace").extract[Seq[String]] == Seq(namespace))
+ assert(!(v \ "isTemporary").extract[Boolean])
+ }
+ }
+
+ test("AS JSON applies the LIKE pattern") {
+ sql(s"CREATE VIEW $catalog.$namespace.v_json_match AS SELECT 1 AS x")
+ sql(s"CREATE VIEW $catalog.$namespace.v_json_skip AS SELECT 1 AS x")
+ val names = viewNames(
+ showViewsAsJson(s"SHOW VIEWS IN $catalog.$namespace LIKE 'v_json_match'
AS JSON"))
+ assert(names == Seq("v_json_match"), s"pattern did not filter: $names")
+ }
+
+ test("AS JSON excludes non-view table entries") {
+ val table = s"$catalog.$namespace.t_json_not_a_view"
+ sql(s"CREATE VIEW $catalog.$namespace.v_json_only AS SELECT 1 AS x")
+ withTable(table) {
+ sql(s"CREATE TABLE $table (x INT) USING parquet")
+ val names = viewNames(showViewsAsJson(s"SHOW VIEWS IN
$catalog.$namespace AS JSON"))
+ assert(names.contains("v_json_only"), s"v_json_only missing: $names")
+ assert(!names.contains("t_json_not_a_view"), s"non-view leaked: $names")
+ }
+ }
+
+ test("AS JSON emits an empty array when no view matches") {
+ val rows = sql(s"SHOW VIEWS IN $catalog.$namespace LIKE
'v_json_no_such_view' AS JSON")
+ .collect()
+ assert(rows.length == 1)
+ // Asserted literally so that a rename of the `views` key cannot pass
unnoticed.
+ assert(rows.head.getString(0) == """{"views":[]}""")
+ }
+
+ test("AS JSON agrees with the tabular output") {
+ sql(s"CREATE VIEW $catalog.$namespace.v_json_parity AS SELECT 1 AS x")
+ val expected = sql(s"SHOW VIEWS IN $catalog.$namespace").collect()
+ .map(r => (r.getString(0), r.getString(1), r.getBoolean(2))).toSet
+ // mkString(".") matches QuotingUtils.quoted only because these namespaces
are single-part;
Review Comment:
**Nit (P3):** `QuotingUtils.quoted` is
`namespace.map(quoteIfNeeded).mkString(".")`, and `quoteIfNeeded` adds
backticks only to parts that do not match `^[a-zA-Z_][a-zA-Z0-9_]*`. The
dot-join therefore equals the tabular `namespace` column whenever no part needs
quoting, whatever the number of parts: `[ns1, ns2]` renders as `ns1.ns2` both
ways, while a single-part `my-db` renders as `` `my-db` `` in the tabular
output. As written, the comment would steer someone away from reusing this
check for nested v2 namespaces, where it works, and implies that any
single-part name is safe, which it is not. Rewording the first line keeps the
second line's caveat accurate:
```suggestion
// mkString(".") matches QuotingUtils.quoted only because no namespace
part needs quoting;
```
**Verification:**
- **Inspection:** The comment's stated condition agrees with
QuotingUtils.quoted: namespaces whose parts all need no quoting compare equal
regardless of how many parts they have, and any part that needs backtick
quoting breaks the comparison.
##########
sql/core/src/test/scala/org/apache/spark/sql/execution/command/ShowViewsSuiteBase.scala:
##########
@@ -64,4 +79,72 @@ 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.
+ // Uses a literal set, not `names` (which is derived from `views` itself
and would match
+ // everything, silently disabling this scoping).
+ val createdNames = Set("v_json_a", "v_json_b")
+ val created = views.filter(v => createdNames.contains((v \
"viewName").extract[String]))
+ created.foreach { v =>
+ assert((v \ "namespace").extract[Seq[String]] == Seq(namespace))
+ assert(!(v \ "isTemporary").extract[Boolean])
+ }
+ }
+
+ test("AS JSON applies the LIKE pattern") {
+ sql(s"CREATE VIEW $catalog.$namespace.v_json_match AS SELECT 1 AS x")
+ sql(s"CREATE VIEW $catalog.$namespace.v_json_skip AS SELECT 1 AS x")
+ val names = viewNames(
+ showViewsAsJson(s"SHOW VIEWS IN $catalog.$namespace LIKE 'v_json_match'
AS JSON"))
Review Comment:
**Nit (P3):** Both AS JSON pattern tests use exact view names
(`'v_json_match'` here and `'v_json_no_such_view'` below). On a `ViewCatalog`
catalog, `ShowViewsJsonCommand` filters with its own
`StringUtils.filterPattern` call (`ShowViewsJsonCommand.scala:87`). An exact
name selects the same views under the documented pattern semantics (`*`
wildcard, `|` alternation, case-insensitive match) as under plain name
equality. So if that call were reduced to `ident.name == p`, both tests would
still pass in the v2 suite, while `LIKE 'v_json_m*'` would silently return
`{"views":[]}`. The session-catalog branch hands the pattern to
`SessionCatalog.listViews`, whose wildcard handling `show-views.sql` already
covers, so the gap is in the v2 copy. A pattern that mixes a case variant, a
wildcard, and an alternation keeps the expected `Seq("v_json_match")` in both
suites and fails under that regression:
```suggestion
showViewsAsJson(s"SHOW VIEWS IN $catalog.$namespace LIKE
'V_JSON_M*|no_such_view' AS JSON"))
```
**Verification:**
- **Regression:** On both a session-catalog namespace and a ViewCatalog
namespace, SHOW VIEWS ... LIKE <pattern> AS JSON with a pattern that combines a
wildcard, a case variant and an alternation lists the matching view and
excludes the non-matching one, and the assertion fails when the ViewCatalog
filter is replaced by plain name equality.
--
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]
