ganeshashree commented on code in PR #59828:
URL: https://github.com/apache/spark/pull/59828#discussion_r4419878189
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -1314,6 +1314,57 @@ trait ImplicitlyFormattedAsJson extends Expression {
def emitsImplicitJsonText: Boolean = true
}
+object ImplicitlyFormattedAsJson {
+ /**
+ * Whether `e` lexically emits JSON text -- i.e. carries an implicit
SQL/JSON `FORMAT JSON` and
+ * should be spliced raw rather than quoted (a nested JSON constructor, or
`JSON_QUERY` under
+ * `KEEP QUOTES`; see [[ImplicitlyFormattedAsJson]]). Sees through a
value-preserving `Collate`
+ * (a postfix `... COLLATE c` only annotates the collation of the JSON text,
e.g.
+ * `JSON_ARRAY(JSON_ARRAY(1) COLLATE UTF8_LCASE)`), but deliberately NOT
through a `Cast`:
+ * `AstBuilder`/`JsonArray.sql` use a `CAST(... AS STRING)` wrapper
specifically to detach a value
+ * from implicit `FORMAT JSON`, so unwrapping it would defeat that
neutralization. The routed/
+ * qualified builders use [[splicesThroughCollationCast]], which
additionally sees through an
+ * analyzer collation `Cast`.
+ */
+ @scala.annotation.tailrec
+ def isImplicitlyJson(e: Expression): Boolean = e match {
+ case c: Collate => isImplicitlyJson(c.child)
+ case i: ImplicitlyFormattedAsJson => i.emitsImplicitJsonText
+ case _ => false
+ }
+
+ /**
+ * The routed/qualified splice decision made by `JsonArrayExpressionBuilder`
/
+ * `JsonObjectExpressionBuilder` on a resolved argument. Like
[[isImplicitlyJson]], but also sees
+ * through an analyzer-inserted collation `Cast`: under an object/view
default collation,
+ * `ApplyDefaultCollation` wraps a nested producer in a `Cast` to the
default `StringType` before
+ * the builder runs, which would otherwise hide the producer and quote it
(unlike the direct
+ * grammar path, which freezes the decision at parse time). Only a genuine
collation change is
Review Comment:
Done
##########
sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala:
##########
@@ -954,6 +1226,23 @@ class JsonArraySuite extends QueryTest with
SharedSparkSession {
}
}
+ test("SPARK-59243: routed/qualified nested JSON_ARRAY splices under a
default collation") {
+ // Both columns resolve through the built-in builder (not the direct
grammar path): col a via
+ // the qualified outer call, col b via the qualified nested call.
`ApplyDefaultCollation` wraps
+ // the nested producer in a collation Cast; the builder must still see
through it (an analyzer
+ // Cast, not a user one) and splice, so the result matches the UTF8_BINARY
case rather than
+ // quoting.
+ withSQLConf(SQLConf.OBJECT_LEVEL_COLLATIONS_ENABLED.key -> "true") {
+ withTable("t") {
+ sql(
+ """CREATE TABLE t DEFAULT COLLATION UTF8_LCASE AS
Review Comment:
Done
##########
sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala:
##########
@@ -549,11 +579,11 @@ class JsonArraySuite extends QueryTest with
SharedSparkSession {
checkAnswer(sql("SELECT json_array(*) FROM VALUES (1, 'x') AS t(a,
b)"), Row("shadowed"))
// The clause-bearing form is not a function call, so it stays the
built-in constructor.
checkAnswer(sql("SELECT json_array('x' NULL ON NULL)"),
Row("""["x"]"""))
- // Nested JSON-producing children stay on the direct-construction
path, so they are not
- // shadowed. This preserves JSON_ARRAY's parse-time splice decisions.
- checkAnswer(sql("SELECT json_array(json_array(1))"), Row("[[1]]"))
+ // A nested clause-free call routes too, so it is shadowed like a flat
call (see the
+ // SPARK-59243 nested-shadowing test). Qualifying every level keeps
the built-in splice.
+ checkAnswer(sql("SELECT builtin.json_array(builtin.json_array(1))"),
Row("[[1]]"))
checkAnswer(
- sql("""SELECT json_array(json_query('{"a":{"x":1}}', '$.a'))"""),
+ sql("""SELECT builtin.json_array(builtin.json_query('{"a":{"x":1}}',
'$.a'))"""),
Review Comment:
Done
##########
sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala:
##########
@@ -646,23 +676,176 @@ class JsonArraySuite extends QueryTest with
SharedSparkSession {
checkAnswer(sql("SELECT system.builtin.json_array(1, 'x')"),
Row("""[1,"x"]"""))
}
- test("a nested JSON-producing argument through a routed JSON_ARRAY call is
quoted, not spliced") {
- // A routed call carries no lexical FORMAT JSON, so a nested
JSON-producing argument (a JSON
- // constructor like json_array or a path function like json_query) is
quoted as a plain value,
- // unlike the JSON_ARRAY(...) grammar which splices it (see the unqualified
- // `json_array(json_array(1))` -> `[[1]]` cases above). The outer call
routes whenever it sees
- // no lexical implicit-JSON element: `JsonArray.isImplicitlyJson`
recognizes only a
- // grammar-built nested JSON expression, so a qualified outer call and an
unqualified call over
- // a qualified nested JSON expression (whose child is still unresolved)
both route and quote.
- // Only a fully unqualified nested JSON expression stays on the direct
grammar path; splicing
- // through a routed call is left as a follow-up.
- checkAnswer(sql("SELECT builtin.json_array(json_array(1))"),
Row("""["[1]"]"""))
- checkAnswer(sql("SELECT system.builtin.json_array(json_array(1))"),
Row("""["[1]"]"""))
- checkAnswer(sql("SELECT builtin.json_array(json_array(1), 2)"),
Row("""["[1]",2]"""))
- checkAnswer(sql("SELECT json_array(builtin.json_array(1))"),
Row("""["[1]"]"""))
+ test("SPARK-59243: a nested JSON constructor through a routed JSON_ARRAY
call is spliced") {
+ // A routed or qualified built-in JSON_ARRAY splices a lexically-nested
JSON constructor
+ // argument, matching the JSON_ARRAY(...) grammar
(`json_array(json_array(1))` -> `[[1]]`).
+ // The built-in builder derives the FORMAT JSON flag from the resolved
argument via
+ // `ImplicitlyFormattedAsJson.splicesThroughCollationCast`, so a qualified
outer call and an
+ // unqualified call over a qualified nested constructor both splice,
matching the fully
+ // unqualified grammar form.
+ checkAnswer(sql("SELECT builtin.json_array(json_array(1))"), Row("[[1]]"))
+ checkAnswer(sql("SELECT system.builtin.json_array(json_array(1))"),
Row("[[1]]"))
+ checkAnswer(sql("SELECT builtin.json_array(json_array(1), 2)"),
Row("[[1],2]"))
+ checkAnswer(sql("SELECT json_array(builtin.json_array(1))"), Row("[[1]]"))
+ checkAnswer(sql("SELECT json_array(json_array(0),
builtin.json_array(1))"), Row("[[0],[1]]"))
checkAnswer(
sql("""SELECT builtin.json_array(json_query('{"a":{"x":1}}', '$.a'))"""),
- Row("""["{\"x\":1}"]"""))
+ Row("""[{"x":1}]"""))
+ // A user CAST still detaches FORMAT JSON, so the element stays quoted
(the routed splice check
+ // sees through an analyzer collation Cast, but not a user `CAST(... AS
STRING)`).
+ checkAnswer(sql("SELECT builtin.json_array(CAST(json_array(1) AS
STRING))"), Row("""["[1]"]"""))
+ }
+
+ test("SPARK-59243: clause-bearing JSON_ARRAY splices qualified nested
constructors") {
+ // Clause-bearing direct forms do not route through function resolution,
but still need to
+ // recover the qualified nested constructor after children resolve.
+ checkAnswer(sql("SELECT json_array(builtin.json_array(1) NULL ON NULL)"),
Row("[[1]]"))
+ checkAnswer(sql("SELECT json_array(builtin.json_array(1) RETURNING
STRING)"), Row("[[1]]"))
+ checkAnswer(
+ sql("SELECT json_array(builtin.json_array(1) FORMAT JSON NULL ON NULL)"),
+ Row("[[1]]"))
+ checkAnswer(
+ sql("SELECT json_array(CAST(builtin.json_array(1) AS STRING) NULL ON
NULL)"),
+ Row("""["[1]"]"""))
+
+ Seq(false, true).foreach { singlePass =>
+ withSQLConf(
+ SQLConf.ANALYZER_SINGLE_PASS_RESOLVER_ENABLED.key ->
singlePass.toString) {
+ val analyzed =
+ sql("SELECT json_array(builtin.json_array(1) FORMAT JSON NULL ON
NULL)")
+ .queryExecution.analyzed
+ assert(analyzed.resolved, s"singlePass=$singlePass")
+ val arrays = analyzed.expressions.flatMap(_.collect { case j:
JsonArray => j })
+ val outer = arrays.find(_.nullBehavior ==
JsonConstructorNullBehavior.Null).get
+ assert(outer.formatJson == Seq(true), s"singlePass=$singlePass")
+ assert(outer.needsValidation == Seq(false), s"singlePass=$singlePass")
+ }
+ }
+ }
+
+ test("SPARK-59243: routed JSON_ARRAY splice round-trips through canonical
SQL") {
+ // The analyzed expression's canonical SQL must reparse to the same
spliced result.
+ Seq(
+ "SELECT builtin.json_array(json_array(1))",
+ "SELECT json_array(builtin.json_array(1))").foreach { query =>
+ val analyzed = sql(query).queryExecution.analyzed
+ val jsonArray = analyzed.expressions.flatMap(_.collect { case j:
JsonArray => j }).head
+ checkAnswer(sql(s"SELECT ${jsonArray.sql}"), Row("[[1]]"))
+ }
+ }
+
+ test("SPARK-59243: spliced nested JSON_ARRAY canonical SQL stays built-in
under a shadow") {
+ // Canonical SQL of a resolved built-in must reparse to the built-in even
under a shadow: the
+ // nested splice now routes, so each clause-free level carries an
ownership clause.
+ val inner = JsonArray(
+ Seq(Literal(1)), Seq(false), Seq(false),
JsonConstructorNullBehavior.Absent, StringType)
+ val canonical = JsonArray(
+ Seq(inner), Seq(true), Seq(false), JsonConstructorNullBehavior.Absent,
StringType).sql
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "second") {
+ try {
+ sql("CREATE TEMPORARY FUNCTION json_array(a STRING) RETURNS STRING
RETURN 'shadowed'")
+ sql("SET PATH = system.session, system.builtin")
+ checkAnswer(sql(s"SELECT $canonical"), Row("[[1]]"))
+ } finally {
+ sql("SET PATH = DEFAULT_PATH")
+ sql("DROP TEMPORARY FUNCTION IF EXISTS json_array")
+ }
+ }
+ }
+
+ test("SPARK-59243: an explicit collation-changing string() cast detaches
splicing") {
+ // `string(... COLLATE c)` is an explicit cast (via the cast alias) that
detaches FORMAT JSON,
+ // so the nested constructor is quoted, not spliced. The
analyzer-collation-cast unwrap in
+ // `splicesThroughCollationCast` must not treat it as an
`ApplyDefaultCollation` cast.
+ checkAnswer(
+ sql("SELECT builtin.json_array(string(json_array(1) COLLATE
UTF8_LCASE))"),
+ Row("""["[1]"]"""))
+ // Also when the cast itself changes the collation: it is still a user
cast, not a
+ // default-collation recoloring, on both the routed and the direct grammar
paths.
+ val nested = "string(json_array(1 RETURNING STRING COLLATE UTF8_LCASE))"
+ checkAnswer(sql(s"SELECT builtin.json_array($nested)"), Row("""["[1]"]"""))
+ checkAnswer(sql(s"SELECT json_array($nested NULL ON NULL)"),
Row("""["[1]"]"""))
+ }
+
+ test("SPARK-59243: a nested clause-free JSON_ARRAY call is shadowed like a
flat call") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "second") {
+ try {
+ sql("CREATE TEMPORARY FUNCTION json_array(a STRING) RETURNS STRING
RETURN 'shadowed'")
+ sql("SET PATH = system.session, system.builtin")
+ // The inner json_array(1) is a grammar element (built-in) producing
"[1]"; the outer
+ // clause-free call routes, so the temp shadows it.
+ checkAnswer(sql("SELECT json_array(json_array(1))"), Row("shadowed"))
+ // Strict single-pass routes the outer clause-free call to the shadow
owner too, but the
+ // owner is a SQL function, which the single-pass analyzer does not
yet resolve: it reaches
+ // that owner (proving the routing) and then aborts with an
+ // ExplicitlyUnsupportedResolverFeature on the SQL function body. The
guard-gated modes fall
+ // back to fixed-point, which produces the shadow result asserted
above.
+ withSQLConf(SQLConf.ANALYZER_SINGLE_PASS_RESOLVER_ENABLED.key ->
"true") {
+ intercept[ExplicitlyUnsupportedResolverFeature] {
+ sql("SELECT json_array(json_array(1))").queryExecution.analyzed
+ }
+ }
+ // Qualifying every level selects Spark's constructor and splices.
+ checkAnswer(sql("SELECT builtin.json_array(builtin.json_array(1))"),
Row("[[1]]"))
+ } finally {
+ sql("SET PATH = DEFAULT_PATH")
+ sql("DROP TEMPORARY FUNCTION IF EXISTS json_array")
+ }
+ }
+ }
+
+ test("SPARK-59243: nested constructors splice in subqueries, grouping, and
INSERT VALUES") {
+ // The finalization rule runs over every operator, not just a top-level
Project.
+ checkAnswer(sql("SELECT (SELECT builtin.json_array(json_array(1)))"),
Row("[[1]]"))
+ checkAnswer(
+ sql("SELECT 1 WHERE '[[1]]' IN (SELECT
json_array(builtin.json_array(1)))"),
+ Row(1))
+ checkAnswer(
+ sql("SELECT json_array(json_array(count(*))) FROM VALUES (1), (2) AS
t(a)"),
+ Row("[[2]]"))
+ // HAVING and ORDER BY are checked separately: combining them with GROUP
BY hits an unrelated
+ // dual-run plan mismatch for any select-list expression.
+ val grouped = "FROM VALUES (1), (2), (2) AS t(a) GROUP BY a"
+ checkAnswer(
+ sql(s"SELECT builtin.json_array(json_array(a)) AS j $grouped " +
+ "HAVING builtin.json_array(json_array(count(*))) = '[[2]]'"),
+ Row("[[2]]"))
+ checkAnswer(
+ sql(s"SELECT builtin.json_array(json_array(a)) AS j $grouped " +
+ "ORDER BY json_array(builtin.json_array(a))"),
+ Seq(Row("[[1]]"), Row("[[2]]")))
+ withTable("t") {
+ sql("CREATE TABLE t (j STRING) USING parquet")
+ sql("INSERT INTO t VALUES (builtin.json_array(json_array(1))),
(json_object('a', " +
+ "builtin.json_object('b', 1)))")
+ checkAnswer(sql("SELECT j FROM t"), Seq(Row("[[1]]"),
Row("""{"a":{"b":1}}""")))
+ }
+ }
+
+ test("SPARK-59243: a persistent function cannot reference a temp shadow via
a nested call") {
+ withSQLConf(
+ SQLConf.PATH_ENABLED.key -> "true",
+ SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "second") {
+ try {
+ sql("CREATE TEMPORARY FUNCTION json_array(a STRING) RETURNS STRING
RETURN 'shadowed'")
+ sql("SET PATH = system.session, system.builtin")
+ // The routed nested call is a reference to the temp shadow, like the
flat call.
+ Seq("json_array(1)", "json_array(json_array(1))").foreach { body =>
+ val e = intercept[AnalysisException] {
+ sql(s"CREATE FUNCTION perm_json_fn() RETURNS STRING RETURN $body")
Review Comment:
Done
--
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]
