ganeshashree commented on code in PR #59828:
URL: https://github.com/apache/spark/pull/59828#discussion_r4422028611
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -1720,6 +1765,144 @@ object JsonArray {
}
}
+/**
+ * A parsed `JSON_ARRAY(...)` / `JSON_OBJECT(...)` whose splice flags are
finalized after child
+ * resolution: parser flags keep explicit `FORMAT JSON` and lexical nested
constructors, and the
+ * resolved children add qualified/routed nested constructors the parser
cannot see.
+ *
+ * [[StockJsonConstructor]]s are owned by the stock built-in.
[[RoutedJsonConstructor]]s are
+ * clause-free calls a same-named routine (or an `injectFunction` replacement)
can shadow; they then
+ * resolve as a plain `prettyName(arguments...)` call.
+ */
+sealed trait UnresolvedJsonConstructor extends Unevaluable with
ImplicitlyFormattedAsJson {
+ def routeThroughResolution: Boolean
+ /** The flattened argument list a routed call passes to a same-named
routine. */
+ def arguments: Seq[Expression] = children
+ /** The stock built-in over the current children, with splice flags
finalized. */
+ def toBuiltin: TimeZoneAwareExpression
+ /** The ordinary call a routed constructor resolves as when a routine owns
the name. */
+ def toUnresolvedFunction: UnresolvedFunction =
CurrentOrigin.withOrigin(origin) {
+ UnresolvedFunction(prettyName, arguments, isDistinct = false)
+ }
+
+ override def dataType: DataType = throw new UnresolvedException("dataType")
+ override def nullable: Boolean = throw new UnresolvedException("nullable")
+ override lazy val resolved: Boolean = false
+ final override val nodePatterns: Seq[TreePattern] =
Seq(UNRESOLVED_JSON_CONSTRUCTOR)
+}
+
+/** An [[UnresolvedJsonConstructor]] the stock built-in owns (clause-bearing
or `FORMAT JSON`). */
Review Comment:
Done
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -1720,6 +1765,144 @@ object JsonArray {
}
}
+/**
+ * A parsed `JSON_ARRAY(...)` / `JSON_OBJECT(...)` whose splice flags are
finalized after child
+ * resolution: parser flags keep explicit `FORMAT JSON` and lexical nested
constructors, and the
+ * resolved children add qualified/routed nested constructors the parser
cannot see.
+ *
+ * [[StockJsonConstructor]]s are owned by the stock built-in.
[[RoutedJsonConstructor]]s are
+ * clause-free calls a same-named routine (or an `injectFunction` replacement)
can shadow; they then
+ * resolve as a plain `prettyName(arguments...)` call.
+ */
+sealed trait UnresolvedJsonConstructor extends Unevaluable with
ImplicitlyFormattedAsJson {
+ def routeThroughResolution: Boolean
+ /** The flattened argument list a routed call passes to a same-named
routine. */
+ def arguments: Seq[Expression] = children
+ /** The stock built-in over the current children, with splice flags
finalized. */
+ def toBuiltin: TimeZoneAwareExpression
+ /** The ordinary call a routed constructor resolves as when a routine owns
the name. */
+ def toUnresolvedFunction: UnresolvedFunction =
CurrentOrigin.withOrigin(origin) {
+ UnresolvedFunction(prettyName, arguments, isDistinct = false)
+ }
+
+ override def dataType: DataType = throw new UnresolvedException("dataType")
+ override def nullable: Boolean = throw new UnresolvedException("nullable")
+ override lazy val resolved: Boolean = false
+ final override val nodePatterns: Seq[TreePattern] =
Seq(UNRESOLVED_JSON_CONSTRUCTOR)
+}
+
+/** An [[UnresolvedJsonConstructor]] the stock built-in owns (clause-bearing
or `FORMAT JSON`). */
+sealed trait StockJsonConstructor
+ extends UnresolvedJsonConstructor
+ with DefaultStringProducingExpression {
+ def returning: DataType
+ final override def routeThroughResolution: Boolean = false
+ // Report the actual RETURNING type (not the default from
`DefaultStringProducingExpression`) so
+ // `ApplyDefaultCollation` sees an explicit `RETURNING STRING COLLATE c` on
this still-unresolved
+ // node and does not recolor it to the object/view default.
+ override def dataType: DataType = returning
+
+ // A parser flag stays set; a resolved child that splices through a
collation cast adds one.
+ // An added splice is a trusted constructor, so it never needs validation.
+ protected def finalizeFlags(
+ parserFlags: Seq[Boolean],
+ needsValidation: Seq[Boolean],
+ values: Seq[Expression]): (Seq[Boolean], Seq[Boolean]) = {
+ val splices =
values.map(ImplicitlyFormattedAsJson.splicesThroughCollationCast)
+ (parserFlags.lazyZip(splices).map(_ || _),
needsValidation.lazyZip(splices).map(_ && !_))
+ }
+}
+
+/**
+ * An [[UnresolvedJsonConstructor]] whose owner is not known until resolution.
Deliberately not a
+ * `DefaultStringProducingExpression`: until a routine is ruled out it stands
for an ordinary call,
+ * so default-collation rules must not cast it (a shadow keeps its own result
type).
+ */
+sealed trait RoutedJsonConstructor extends UnresolvedJsonConstructor {
+ final override def routeThroughResolution: Boolean = true
+}
+
+case class UnresolvedJsonArray(
+ values: Seq[Expression],
+ formatJson: Seq[Boolean],
+ needsValidation: Seq[Boolean],
+ nullBehavior: JsonConstructorNullBehavior,
+ returning: DataType)
+ extends StockJsonConstructor {
+ assert(values.length == formatJson.length && values.length ==
needsValidation.length,
+ "UnresolvedJsonArray requires one formatJson and one needsValidation flag
per value")
+ assert(needsValidation.lazyZip(formatJson).forall((nv, fj) => !nv || fj),
+ "UnresolvedJsonArray needsValidation implies formatJson")
+ override def children: Seq[Expression] = values
+ override def prettyName: String = "json_array"
+ // Only the values are children, so `withNewChildren` (e.g. a
default-collation `Cast` wrapping a
+ // child) preserves the parse-time flags.
+ override protected def withNewChildrenInternal(
+ newChildren: IndexedSeq[Expression]): UnresolvedJsonArray = copy(values
= newChildren)
+
+ override def toBuiltin: JsonArray = {
+ val (finalFormatJson, finalNeedsValidation) = finalizeFlags(formatJson,
needsValidation, values)
+ JsonArray(values, finalFormatJson, finalNeedsValidation, nullBehavior,
returning)
+ }
+}
+
+/** A clause-free `JSON_ARRAY(...)` with a lexically nested constructor (see
[[JsonArray]]). */
+case class UnresolvedRoutedJsonArray(values: Seq[Expression], formatJson:
Seq[Boolean])
+ extends RoutedJsonConstructor {
+ assert(values.length == formatJson.length,
+ "UnresolvedRoutedJsonArray requires one formatJson flag per value")
+ override def children: Seq[Expression] = values
+ override def prettyName: String = "json_array"
+ override protected def withNewChildrenInternal(
+ newChildren: IndexedSeq[Expression]): UnresolvedRoutedJsonArray =
copy(values = newChildren)
+
+ override def toBuiltin: JsonArray = UnresolvedJsonArray(
+ values, formatJson, values.map(_ => false),
JsonConstructorNullBehavior.Absent, StringType
Review Comment:
Done
##########
sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala:
##########
@@ -954,6 +1283,77 @@ 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
Review Comment:
Done
##########
sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala:
##########
@@ -646,23 +681,197 @@ 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]"]"""))
+ // A user cast that changes the collation is told apart from an analyzer
collation Cast only by
+ // its USER_SPECIFIED_CAST tag, and must still detach.
+ checkAnswer(
+ sql("SELECT builtin.json_array(CAST(json_array(1) AS STRING COLLATE
UTF8_LCASE))"),
+ Row("""["[1]"]"""))
+ withSQLConf(SQLConf.OBJECT_LEVEL_COLLATIONS_ENABLED.key -> "true") {
+ withTable("t") {
+ sql("CREATE TABLE t DEFAULT COLLATION UTF8_LCASE AS " +
+ "SELECT builtin.json_array(CAST(json_array(1) AS STRING)) AS a")
+ checkAnswer(spark.table("t"), 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.
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]
