From 389b48a3356fb3bfe7cc633599aff680650fb6f6 Mon Sep 17 00:00:00 2001 From: Ganesha S Date: Mon, 21 Sep 2026 05:32:28 +0000 Subject: [PATCH 1/5] [SPARK-59685][SQL] Keep clause-free SQL/JSON constructor canonical SQL bound to the built-in A clause-free JSON_VALUE / JSON_QUERY / JSON_EXISTS / JSON_ARRAY call routes through function resolution (SPARK-59144), but these built-ins' canonical `sql` drops default clauses, so a call that is the built-in only because of a default clause renders clause-free SQL that, reparsed under a shadowing PATH, binds a same-named routine instead of the built-in. Render the default clause (RETURNING STRING, or FALSE ON ERROR for JSON_EXISTS) so canonical SQL reparses back to the built-in; `usePrettyExpression` renders the clean clause-free form (dropping the round-trip-only clause and rendering children in place, so nested splice decisions are preserved) and auto-generated column names are unaffected. Generated-by: Claude Code (Opus 4.8) Co-authored-by: Isaac --- .../expressions/jsonExpressions.scala | 93 +++++++++++--- .../spark/sql/catalyst/util/package.scala | 10 ++ .../org/apache/spark/sql/JsonArraySuite.scala | 121 ++++++++++++++++-- .../apache/spark/sql/JsonExistsSuite.scala | 40 ++++++ .../org/apache/spark/sql/JsonQuerySuite.scala | 40 ++++++ .../org/apache/spark/sql/JsonValueSuite.scala | 42 ++++++ 6 files changed, 318 insertions(+), 28 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala index 391b4d3eee2d9..c059db74d5069 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala @@ -687,6 +687,19 @@ object JsonValueBehavior { * }}} */ // scalastyle:on line.size.limit + +/** + * A SQL/JSON function whose clause-free call `AstBuilder` routes through function resolution, so + * its canonical `sql` must append the default clause to reparse back to this built-in instead of + * a same-named routine on the SQL PATH. `sqlString`'s `forceBuiltinOwnership` gates that clause + * (true for `sql`, false for the never-reparsed pretty form) and `renderChild` renders each child. + */ +trait RoutedSqlJsonExpression extends Expression { + private[sql] def sqlString( + forceBuiltinOwnership: Boolean, + renderChild: Expression => String): String +} + case class JsonValue( child: Expression, path: String, @@ -701,7 +714,8 @@ case class JsonValue( with TimeZoneAwareExpression with CodegenFallback with ExpectsInputTypes - with QueryErrorsBase { + with QueryErrorsBase + with RoutedSqlJsonExpression { override def nullable: Boolean = true @@ -825,7 +839,12 @@ case class JsonValue( override def prettyName: String = "json_value" - override def sql: String = { + override def sql: String = sqlString(forceBuiltinOwnership = true, _.sql) + + // See [[RoutedSqlJsonExpression]] for `forceBuiltinOwnership` and `renderChild`. + private[sql] def sqlString( + forceBuiltinOwnership: Boolean, + renderChild: Expression => String): String = { // Reference identity, not value equality: an explicitly collated `RETURNING STRING COLLATE // UTF8_BINARY` is a distinct instance that compares `==` to the companion, so `==` would drop // it. Only the companion (by reference) renders nothing -- reached by omission and by a plain @@ -835,7 +854,7 @@ case class JsonValue( def behaviorSQL(b: JsonValueBehavior, default: Option[Expression]): String = b match { case JsonValueBehavior.Null => "NULL" case JsonValueBehavior.Error => "ERROR" - case JsonValueBehavior.Default => s"DEFAULT ${default.get.sql}" + case JsonValueBehavior.Default => s"DEFAULT ${renderChild(default.get)}" } val emptySQL = if (onEmpty == JsonValueBehavior.Null) "" else s" ${behaviorSQL(onEmpty, emptyDefault)} ON EMPTY" @@ -844,7 +863,12 @@ case class JsonValue( // Render the path as a properly escaped string literal so bracket-quoted paths such as // `$['a']` (and any path containing a quote or backslash) round-trip as valid SQL. val pathSQL = Literal(UTF8String.fromString(path), StringType).sql - s"JSON_VALUE(${child.sql}, $pathSQL$returningSQL$emptySQL$errorSQL)" + // Append the default `RETURNING STRING` when the render is otherwise clause-free. + val ownershipClause = + if (forceBuiltinOwnership && returningSQL.isEmpty && emptySQL.isEmpty && errorSQL.isEmpty) { + " RETURNING STRING" + } else "" + s"JSON_VALUE(${renderChild(child)}, $pathSQL$returningSQL$emptySQL$errorSQL$ownershipClause)" } override protected def withNewChildrenInternal( @@ -918,7 +942,8 @@ case class JsonExists( extends UnaryExpression with CodegenFallback with ExpectsInputTypes - with QueryErrorsBase { + with QueryErrorsBase + with RoutedSqlJsonExpression { // The result is NULL only when the input is SQL NULL, or when `UNKNOWN ON ERROR` turns malformed // input into a BOOLEAN NULL. With a non-nullable input and any other ON ERROR behavior the result @@ -979,14 +1004,21 @@ case class JsonExists( override def prettyName: String = "json_exists" - override def sql: String = { + override def sql: String = sqlString(forceBuiltinOwnership = true, _.sql) + + // See [[RoutedSqlJsonExpression]] for `forceBuiltinOwnership` and `renderChild`. + private[sql] def sqlString( + forceBuiltinOwnership: Boolean, + renderChild: Expression => String): String = { val errorSQL = onError match { case JsonExistsBehavior.False => "" // the default case JsonExistsBehavior.True => " TRUE ON ERROR" case JsonExistsBehavior.Unknown => " UNKNOWN ON ERROR" case JsonExistsBehavior.Error => " ERROR ON ERROR" } - s"JSON_EXISTS(${child.sql}, ${toSQLValue(path)}$errorSQL)" + // Append the default `FALSE ON ERROR` when the render is otherwise clause-free. + val ownershipClause = if (forceBuiltinOwnership && errorSQL.isEmpty) " FALSE ON ERROR" else "" + s"JSON_EXISTS(${renderChild(child)}, ${toSQLValue(path)}$errorSQL$ownershipClause)" } override protected def withNewChildInternal(newChild: Expression): JsonExists = @@ -1069,7 +1101,8 @@ case class JsonQuery( with CodegenFallback with ExpectsInputTypes with QueryErrorsBase - with ImplicitlyFormattedAsJson { + with ImplicitlyFormattedAsJson + with RoutedSqlJsonExpression { override def nullable: Boolean = true @@ -1163,7 +1196,12 @@ case class JsonQuery( override def prettyName: String = "json_query" - override def sql: String = { + override def sql: String = sqlString(forceBuiltinOwnership = true, _.sql) + + // See [[RoutedSqlJsonExpression]] for `forceBuiltinOwnership` and `renderChild`. + private[sql] def sqlString( + forceBuiltinOwnership: Boolean, + renderChild: Expression => String): String = { // Reference identity, not value equality, so an explicitly collated `RETURNING STRING COLLATE // UTF8_BINARY` (a distinct instance that compares `==` to the companion) is still rendered. // Only the companion (by reference) renders nothing -- reached by omission and by a plain @@ -1191,7 +1229,16 @@ case class JsonQuery( if (onError == JsonQueryBehavior.Null) "" else s" ${behaviorSQL(onError)} ON ERROR" // Render the path as a properly escaped string literal so bracket-quoted paths round-trip. val pathSQL = Literal(UTF8String.fromString(path), StringType).sql - s"JSON_QUERY(${child.sql}, $pathSQL$returningSQL$wrapperSQL$quotesSQL$emptySQL$errorSQL)" + // Append the default `RETURNING STRING` when the render is otherwise clause-free. + val ownershipClause = + if (forceBuiltinOwnership && returningSQL.isEmpty && wrapperSQL.isEmpty && + quotesSQL.isEmpty && emptySQL.isEmpty && errorSQL.isEmpty) { + " RETURNING STRING" + } else { + "" + } + s"JSON_QUERY(${renderChild(child)}, " + + s"$pathSQL$returningSQL$wrapperSQL$quotesSQL$emptySQL$errorSQL$ownershipClause)" } override protected def withNewChildInternal(newChild: Expression): JsonQuery = @@ -1300,7 +1347,8 @@ case class JsonArray( with ExpectsInputTypes with QueryErrorsBase with DefaultStringProducingExpression - with ImplicitlyFormattedAsJson { + with ImplicitlyFormattedAsJson + with RoutedSqlJsonExpression { // `formatJson(i)` freezes whether element `i` is spliced raw (vs quoted); `needsValidation(i)` // freezes whether its raw text is arbitrary user input that must be JSON-validated at eval (an @@ -1553,7 +1601,12 @@ case class JsonArray( override def prettyName: String = "json_array" - override def sql: String = { + override def sql: String = sqlString(forceBuiltinOwnership = true, _.sql) + + // See [[RoutedSqlJsonExpression]] for `forceBuiltinOwnership` and `renderChild`. + private[sql] def sqlString( + forceBuiltinOwnership: Boolean, + renderChild: Expression => String): String = { val valuesSQL = values.zip(formatJson).map { case (v, isJson) => // Emit SQL that reparses to the same splice/quote decision as the frozen `formatJson` flag. // The parser splices a value iff it is an explicit `FORMAT JSON` or a lexically-nested JSON @@ -1577,11 +1630,11 @@ case class JsonArray( } val transitivelyImplicit = JsonArray.isImplicitlyJson(v) if (isJson && !directlyImplicit) { - s"${v.sql} FORMAT JSON" + s"${renderChild(v)} FORMAT JSON" } else if (!isJson && transitivelyImplicit) { - s"CAST(${v.sql} AS STRING)" + s"CAST(${renderChild(v)} AS STRING)" } else { - v.sql + renderChild(v) } }.mkString(", ") // Reference identity, not value equality: an explicitly collated `RETURNING STRING COLLATE @@ -1594,7 +1647,15 @@ case class JsonArray( case JsonConstructorNullBehavior.Null => " NULL ON NULL" case JsonConstructorNullBehavior.Absent => "" } - s"JSON_ARRAY($valuesSQL$nullSQL$returningSQL)" + // Append the default `RETURNING STRING` when the render is otherwise clause-free. A spliced + // element (an explicit `FORMAT JSON`, or a nested constructor that reparses as implicit JSON) + // already forces the direct grammar branch, so it needs no clause. + val ownershipClause = + if (forceBuiltinOwnership && !formatJson.exists(identity) && nullSQL.isEmpty && + returningSQL.isEmpty) { + " RETURNING STRING" + } else "" + s"JSON_ARRAY($valuesSQL$nullSQL$returningSQL$ownershipClause)" } override protected def withNewChildrenInternal( diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/package.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/package.scala index 2a412c538b0c1..75a1d6a046081 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/package.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/package.scala @@ -129,6 +129,16 @@ package object util extends Logging { ) case c: Cast if !c.containsTag(Cast.USER_SPECIFIED_CAST) => PrettyAttribute(usePrettyExpression(c.child, shouldTrimTempResolvedColumn).sql, c.dataType) + case j: RoutedSqlJsonExpression => + // Column names are for display and never reparsed, so render the clean clause-free form, not + // the canonical `sql` that appends a round-trip-only default clause. Render children in place + // rather than substituting the node's children, so the raw-vs-quoted splice decisions (which + // key off each child's identity) are unchanged and no synthetic `FORMAT JSON` leaks in. + PrettyAttribute( + j.sqlString( + forceBuiltinOwnership = false, + usePrettyExpression(_, shouldTrimTempResolvedColumn).sql), + j.dataType) case p: PythonFuncExpression => PrettyPythonUDF(p.name, p.dataType, p.children) // Present a transpiled UDF exactly like the UDF it wraps, so auto-generated // column names stay `f(a)` whether or not transpilation engages (the node diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala index 2e8da431883f7..57fff7378bac3 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala @@ -342,25 +342,36 @@ class JsonArraySuite extends QueryTest with SharedSparkSession { // as-is: reparse re-derives implicit FORMAT JSON. val spliced = JsonArray( Seq(inner), Seq(true), Seq(false), JsonConstructorNullBehavior.Absent, StringType) - assert(spliced.sql == "JSON_ARRAY(JSON_ARRAY(1))") + // The outer splices, so it is on the direct grammar path already; the inner built-in renders a + // default `RETURNING STRING` so its own clause-free form would not reparse into a + // shadowing routine. + assert(spliced.sql == "JSON_ARRAY(JSON_ARRAY(1 RETURNING STRING))") // But a constructor inlined into a quoted (formatJson = false) position must be wrapped so // reparse keeps it quoted -- otherwise ["[1]"] would round-trip to [[1]]. val quoted = JsonArray( Seq(inner), Seq(false), Seq(false), JsonConstructorNullBehavior.Absent, StringType) - assert(quoted.sql == "JSON_ARRAY(CAST(JSON_ARRAY(1) AS STRING))") + assert(quoted.sql == + "JSON_ARRAY(CAST(JSON_ARRAY(1 RETURNING STRING) AS STRING) RETURNING STRING)") } test("emitted SQL reparses and evaluates with raw-vs-quoted semantics preserved") { - // The .sql renderings above are round-trip contracts: reparsing and evaluating them must + // The .sql renderings above are round-trip contracts: reparsing and evaluating the actual + // emitted SQL -- including the default RETURNING STRING clauses it now carries -- must // reproduce the original splicing. A bare nested constructor stays spliced; a cast-neutralized // one stays quoted. - checkAnswer(sql("SELECT JSON_ARRAY(JSON_ARRAY(1))"), Row("[[1]]")) - checkAnswer(sql("SELECT JSON_ARRAY(CAST(JSON_ARRAY(1) AS STRING))"), Row("""["[1]"]""")) - // An explicit FORMAT JSON string literal round-trips through the emitted SQL too. + val inner = JsonArray( + Seq(Literal(1)), Seq(false), Seq(false), JsonConstructorNullBehavior.Absent, StringType) val spliced = JsonArray( + Seq(inner), Seq(true), Seq(false), JsonConstructorNullBehavior.Absent, StringType) + checkAnswer(sql(s"SELECT ${spliced.sql}"), Row("[[1]]")) + val quoted = JsonArray( + Seq(inner), Seq(false), Seq(false), JsonConstructorNullBehavior.Absent, StringType) + checkAnswer(sql(s"SELECT ${quoted.sql}"), Row("""["[1]"]""")) + // An explicit FORMAT JSON string literal round-trips through the emitted SQL too. + val splicedLiteral = JsonArray( Seq(Literal("[1,2]")), Seq(true), Seq(true), JsonConstructorNullBehavior.Absent, StringType) - assert(spliced.sql == "JSON_ARRAY('[1,2]' FORMAT JSON)") - checkAnswer(sql(s"SELECT ${spliced.sql}"), Row("[[1,2]]")) + assert(splicedLiteral.sql == "JSON_ARRAY('[1,2]' FORMAT JSON)") + checkAnswer(sql(s"SELECT ${splicedLiteral.sql}"), Row("[[1,2]]")) } test("SQL forces FORMAT JSON for a spliced value whose child is not a bare constructor") { @@ -386,24 +397,40 @@ class JsonArraySuite extends QueryTest with SharedSparkSession { val keep = jsonQuery(JsonQueryQuotes.Keep) val splicedKeep = JsonArray( Seq(keep), Seq(true), Seq(false), JsonConstructorNullBehavior.Absent, StringType) - assert(splicedKeep.sql == """JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a'))""") + assert(splicedKeep.sql == + """JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a' RETURNING STRING))""") // A KEEP QUOTES JSON_QUERY inlined into a quoted position must be neutralized with a cast so // reparse keeps it quoted rather than re-deriving implicit FORMAT JSON. val quotedKeep = JsonArray( Seq(keep), Seq(false), Seq(false), JsonConstructorNullBehavior.Absent, StringType) - assert(quotedKeep.sql == """JSON_ARRAY(CAST(JSON_QUERY('{"a":{"x":1}}', '$.a') AS STRING))""") + assert(quotedKeep.sql == + """JSON_ARRAY(CAST(JSON_QUERY('{"a":{"x":1}}', """ + + """'$.a' RETURNING STRING) AS STRING) RETURNING STRING)""") // OMIT QUOTES emits an ordinary string, so it is not implicit: in a quoted position it renders // as-is, and in a spliced position it must render an explicit FORMAT JSON (it does not // round-trip implicitly). val omit = jsonQuery(JsonQueryQuotes.Omit) val quotedOmit = JsonArray( Seq(omit), Seq(false), Seq(false), JsonConstructorNullBehavior.Absent, StringType) - assert(quotedOmit.sql == """JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a' OMIT QUOTES))""") + assert(quotedOmit.sql == + """JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a' OMIT QUOTES) RETURNING STRING)""") val splicedOmit = JsonArray( Seq(omit), Seq(true), Seq(true), JsonConstructorNullBehavior.Absent, StringType) assert( splicedOmit.sql == """JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a' OMIT QUOTES) FORMAT JSON)""") + // Each emitted rendering (with the default RETURNING STRING it now carries) must reparse and + // evaluate the same as its clause-free equivalent, i.e. the added clause is semantically inert. + checkAnswer(sql(s"SELECT ${splicedKeep.sql}"), + sql("""SELECT JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a'))""").collect().toSeq) + checkAnswer(sql(s"SELECT ${quotedKeep.sql}"), + sql("""SELECT JSON_ARRAY(CAST(JSON_QUERY('{"a":{"x":1}}', '$.a') AS STRING))""") + .collect().toSeq) + checkAnswer(sql(s"SELECT ${quotedOmit.sql}"), + sql("""SELECT JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a' OMIT QUOTES))""").collect().toSeq) + checkAnswer(sql(s"SELECT ${splicedOmit.sql}"), + sql("""SELECT JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a' OMIT QUOTES) FORMAT JSON)""") + .collect().toSeq) } test("SQL preserves an explicit collated RETURNING on a spliced nested JSON_QUERY") { @@ -431,7 +458,9 @@ class JsonArraySuite extends QueryTest with SharedSparkSession { // The omitted default is the companion StringType (by reference) and renders no RETURNING. val default = JsonArray( Seq(Literal(1)), Seq(false), Seq(false), JsonConstructorNullBehavior.Absent, StringType) - assert(default.sql == "JSON_ARRAY(1)") + // A clause-free built-in renders the default `RETURNING STRING` so its canonical SQL stays + // bound to the built-in on reparse. + assert(default.sql == "JSON_ARRAY(1 RETURNING STRING)") } test("a constant JSON_ARRAY is foldable unless it has an explicit FORMAT JSON") { @@ -543,6 +572,74 @@ class JsonArraySuite extends QueryTest with SharedSparkSession { } } + test("SPARK-59685: default-clause JSON_ARRAY canonical SQL reparses to the built-in under a " + + "shadowing PATH") { + withSQLConf( + SQLConf.PATH_ENABLED.key -> "true", + SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "second") { + try { + sql("CREATE TEMPORARY FUNCTION json_array(a INT) RETURNS STRING RETURN 'shadowed'") + sql("SET PATH = system.session, system.builtin") + // A built-in JSON_ARRAY whose only clause is the default RETURNING STRING: the clause makes + // it the built-in even under the shadow, but its canonical `sql` would drop the default. + val jsonArray = sql("SELECT json_array(1 RETURNING STRING)") + .queryExecution.analyzed.expressions + .flatMap(_.collect { case ja: JsonArray => ja }).head + // The rendering must reparse back to the built-in, not the same-named routine on the PATH. + val reparsed = sql(s"SELECT ${jsonArray.sql}") + assert(reparsed.queryExecution.analyzed.expressions + .exists(_.exists(_.isInstanceOf[JsonArray])), + s"canonical SQL bound the shadow instead of the built-in: ${jsonArray.sql}") + checkAnswer(reparsed, Row("[1]")) + } finally { + sql("SET PATH = DEFAULT_PATH") + sql("DROP TEMPORARY FUNCTION IF EXISTS json_array") + } + } + } + + test("SPARK-59685: zero-arg default JSON_ARRAY canonical SQL reparses to the built-in under a " + + "shadowing PATH") { + withSQLConf( + SQLConf.PATH_ENABLED.key -> "true", + SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "second") { + try { + sql("CREATE TEMPORARY FUNCTION json_array() RETURNS STRING RETURN 'shadowed'") + sql("SET PATH = system.session, system.builtin") + // A zero-arg built-in JSON_ARRAY renders the distinct clause-only `JSON_ARRAY( RETURNING + // STRING)`: it has no values, so the default clause is its sole marker on reparse. + val jsonArray = sql("SELECT json_array(RETURNING STRING)") + .queryExecution.analyzed.expressions + .flatMap(_.collect { case ja: JsonArray => ja }).head + // The rendering must reparse back to the built-in, not the same-named routine on the PATH. + val reparsed = sql(s"SELECT ${jsonArray.sql}") + assert(reparsed.queryExecution.analyzed.expressions + .exists(_.exists(_.isInstanceOf[JsonArray])), + s"canonical SQL bound the shadow instead of the built-in: ${jsonArray.sql}") + checkAnswer(reparsed, Row("[]")) + } finally { + sql("SET PATH = DEFAULT_PATH") + sql("DROP TEMPORARY FUNCTION IF EXISTS json_array") + } + } + } + + test("SPARK-59685: a default JSON_ARRAY keeps a clean auto-generated column name") { + val name = sql("SELECT json_array(1)").schema.head.name + assert(!name.contains("RETURNING"), s"column name leaked the ownership clause: $name") + } + + test("SPARK-59685: a nested JSON constructor keeps a clean auto-generated column name") { + // Children are rendered in place for display names, so a nested constructor neither leaks the + // round-trip RETURNING clause nor gains a synthetic FORMAT JSON that the outer would emit if + // the child were flattened to a plain attribute first. + val splicedArray = sql("SELECT json_array(json_array(1, 2), 3)").schema.head.name + assert(splicedArray === "JSON_ARRAY(JSON_ARRAY(1, 2), 3)", s"unexpected name: $splicedArray") + val nestedQuery = sql("""SELECT json_array(json_query('{"a":1}', '$.a'))""").schema.head.name + assert(!nestedQuery.contains("RETURNING") && !nestedQuery.contains("FORMAT JSON"), + s"nested name leaked a round-trip-only clause: $nestedQuery") + } + test("qualified plain JSON_ARRAY resolves to the built-in constructor") { checkAnswer(sql("SELECT builtin.json_array(1, 'x')"), Row("""[1,"x"]""")) checkAnswer(sql("SELECT system.builtin.json_array(1, 'x')"), Row("""[1,"x"]""")) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala index cea519a115cb9..a43dece658a50 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala @@ -61,6 +61,46 @@ class JsonExistsSuite extends QueryTest with SharedSparkSession { } } + test("SPARK-59685: default-clause JSON_EXISTS canonical SQL reparses to the built-in under a " + + "shadowing PATH") { + withSQLConf( + SQLConf.PATH_ENABLED.key -> "true", + SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "second") { + try { + sql("CREATE TEMPORARY FUNCTION json_exists(a STRING, b STRING) RETURNS BOOLEAN " + + "RETURN false") + sql("SET PATH = system.session, system.builtin") + // A built-in JSON_EXISTS whose only clause is the default FALSE ON ERROR: the clause makes + // it the built-in even under the shadow, but its canonical `sql` would drop the default. + val jsonExists = sql(s"SELECT json_exists('$doc', '$$.addr.city' FALSE ON ERROR)") + .queryExecution.analyzed.expressions + .flatMap(_.collect { case je: JsonExists => je }).head + // The rendering must reparse back to the built-in, not the same-named routine on the PATH. + val reparsed = sql(s"SELECT ${jsonExists.sql}") + assert(reparsed.queryExecution.analyzed.expressions + .exists(_.exists(_.isInstanceOf[JsonExists])), + s"canonical SQL bound the shadow instead of the built-in: ${jsonExists.sql}") + checkAnswer(reparsed, Row(true)) + } finally { + sql("SET PATH = DEFAULT_PATH") + sql("DROP TEMPORARY FUNCTION IF EXISTS json_exists") + } + } + } + + test("SPARK-59685: a default JSON_EXISTS keeps a clean auto-generated column name") { + val name = sql(s"SELECT json_exists('$doc', '$$.addr.city')").schema.head.name + assert(!name.contains("ON ERROR"), s"column name leaked the ownership clause: $name") + } + + test("SPARK-59685: a nested SQL/JSON source keeps a clean auto-generated column name") { + // The JSON source is itself a routed built-in constructor; it is rendered in place, so its + // clause-free display form must not leak the round-trip RETURNING clause into the name. + val name = sql("SELECT json_exists(json_array(1), '$[0]')").schema.head.name + assert(!name.contains("RETURNING"), s"nested source leaked the ownership clause: $name") + assert(name.contains("JSON_ARRAY(1)"), s"unexpected name: $name") + } + test("returns BOOLEAN") { assert(sql(s"SELECT json_exists('$doc', '$$.id')").schema.head.dataType === BooleanType) } diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonQuerySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonQuerySuite.scala index d52a0b374e692..25dc0d7193941 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonQuerySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonQuerySuite.scala @@ -66,6 +66,46 @@ class JsonQuerySuite extends QueryTest with SharedSparkSession { } } + test("SPARK-59685: default-clause JSON_QUERY canonical SQL reparses to the built-in under a " + + "shadowing PATH") { + withSQLConf( + SQLConf.PATH_ENABLED.key -> "true", + SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "second") { + try { + sql("CREATE TEMPORARY FUNCTION json_query(a STRING, b STRING) RETURNS STRING " + + "RETURN 'shadowed'") + sql("SET PATH = system.session, system.builtin") + // A built-in JSON_QUERY whose only clause is the default RETURNING STRING: the clause makes + // it the built-in even under the shadow, but its canonical `sql` would drop the default. + val jsonQuery = sql(s"SELECT json_query('$doc', '$$.addr' RETURNING STRING)") + .queryExecution.analyzed.expressions + .flatMap(_.collect { case jq: JsonQuery => jq }).head + // The rendering must reparse back to the built-in, not the same-named routine on the PATH. + val reparsed = sql(s"SELECT ${jsonQuery.sql}") + assert(reparsed.queryExecution.analyzed.expressions + .exists(_.exists(_.isInstanceOf[JsonQuery])), + s"canonical SQL bound the shadow instead of the built-in: ${jsonQuery.sql}") + checkAnswer(reparsed, Row("""{"city":"NYC"}""")) + } finally { + sql("SET PATH = DEFAULT_PATH") + sql("DROP TEMPORARY FUNCTION IF EXISTS json_query") + } + } + } + + test("SPARK-59685: a default JSON_QUERY keeps a clean auto-generated column name") { + val name = sql(s"SELECT json_query('$doc', '$$.addr')").schema.head.name + assert(!name.contains("RETURNING"), s"column name leaked the ownership clause: $name") + } + + test("SPARK-59685: a nested SQL/JSON source keeps a clean auto-generated column name") { + // The JSON source is itself a routed built-in constructor; it is rendered in place, so its + // clause-free display form must not leak the round-trip RETURNING clause into the name. + val name = sql("SELECT json_query(json_array(1), '$[0]')").schema.head.name + assert(!name.contains("RETURNING"), s"nested source leaked the ownership clause: $name") + assert(name.contains("JSON_ARRAY(1)"), s"unexpected name: $name") + } + test("extract an object or array as verbatim JSON text") { checkAnswer(sql(s"SELECT json_query('$doc', '$$.addr')"), Row("""{"city":"NYC"}""")) checkAnswer(sql(s"SELECT json_query('$doc', '$$.tags')"), Row("""["x","y"]""")) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala index e342d01c160c9..7f8897225ee47 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala @@ -61,6 +61,48 @@ class JsonValueSuite extends QueryTest with SharedSparkSession { } } + test("SPARK-59685: default-clause JSON_VALUE canonical SQL reparses to the built-in under a " + + "shadowing PATH") { + withSQLConf( + SQLConf.PATH_ENABLED.key -> "true", + SQLConf.SESSION_FUNCTION_RESOLUTION_ORDER.key -> "second") { + try { + sql("CREATE TEMPORARY FUNCTION json_value(a STRING, b STRING) RETURNS STRING " + + "RETURN 'shadowed'") + sql("SET PATH = system.session, system.builtin") + // A built-in JSON_VALUE whose only clause is the default RETURNING STRING: the clause makes + // it the built-in even under the shadow, but its canonical `sql` would drop the default. + val jsonValue = sql(s"SELECT json_value('$doc', '$$.name' RETURNING STRING)") + .queryExecution.analyzed.expressions + .flatMap(_.collect { case jv: JsonValue => jv }).head + // The rendering must reparse back to the built-in, not the same-named routine on the PATH. + val reparsed = sql(s"SELECT ${jsonValue.sql}") + assert(reparsed.queryExecution.analyzed.expressions + .exists(_.exists(_.isInstanceOf[JsonValue])), + s"canonical SQL bound the shadow instead of the built-in: ${jsonValue.sql}") + checkAnswer(reparsed, Row("Ada")) + } finally { + sql("SET PATH = DEFAULT_PATH") + sql("DROP TEMPORARY FUNCTION IF EXISTS json_value") + } + } + } + + test("SPARK-59685: a default JSON_VALUE keeps a clean auto-generated column name") { + // The canonical `sql` forces the default RETURNING STRING so it stays bound to the built-in on + // reparse, but the display (column) name is never reparsed and must not leak that clause. + val name = sql(s"SELECT json_value('$doc', '$$.name')").schema.head.name + assert(!name.contains("RETURNING"), s"column name leaked the ownership clause: $name") + } + + test("SPARK-59685: a nested SQL/JSON source keeps a clean auto-generated column name") { + // The JSON source is itself a routed built-in constructor; it is rendered in place, so its + // clause-free display form must not leak the round-trip RETURNING clause into the name. + val name = sql("SELECT json_value(json_array(1), '$[0]')").schema.head.name + assert(!name.contains("RETURNING"), s"nested source leaked the ownership clause: $name") + assert(name.contains("JSON_ARRAY(1)"), s"unexpected name: $name") + } + test("extract a scalar value as STRING by default") { checkAnswer(sql(s"SELECT json_value('$doc', '$$.name')"), Row("Ada")) // Numbers and booleans come back as their JSON text under the default STRING RETURNING. From 39188c87845d0bf51248ac4a886f16576a540447 Mon Sep 17 00:00:00 2001 From: Ganesha S Date: Wed, 23 Sep 2026 04:32:57 +0000 Subject: [PATCH 2/5] [SPARK-59685][SQL] Cover routed SQL/JSON DEFAULT child in column-name test Assert a routed built-in (JSON_ARRAY(1)) in JSON_VALUE's DEFAULT ON EMPTY / ON ERROR positions keeps a clean auto-generated column name, guarding the renderChild path the earlier tests missed. Generated-by: Claude Code (Opus 4.8) Co-authored-by: Isaac --- .../scala/org/apache/spark/sql/JsonValueSuite.scala | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala index 7f8897225ee47..8f91373f0b62f 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala @@ -103,6 +103,17 @@ class JsonValueSuite extends QueryTest with SharedSparkSession { assert(name.contains("JSON_ARRAY(1)"), s"unexpected name: $name") } + test("SPARK-59685: a nested SQL/JSON DEFAULT keeps a clean auto-generated column name") { + // A routed built-in constructor in the DEFAULT ... ON EMPTY / ON ERROR position is rendered in + // place too, so its clause-free display form must not leak the round-trip RETURNING clause. + Seq("ON EMPTY", "ON ERROR").foreach { position => + val name = sql(s"SELECT json_value('$doc', '$$.name' DEFAULT json_array(1) $position)") + .schema.head.name + assert(!name.contains("RETURNING"), s"nested DEFAULT leaked the ownership clause: $name") + assert(name.contains("JSON_ARRAY(1)"), s"unexpected name: $name") + } + } + test("extract a scalar value as STRING by default") { checkAnswer(sql(s"SELECT json_value('$doc', '$$.name')"), Row("Ada")) // Numbers and booleans come back as their JSON text under the default STRING RETURNING. From 12195ad349f8760e621e48adcc974b4d250a8ff8 Mon Sep 17 00:00:00 2001 From: Ganesha S Date: Wed, 23 Sep 2026 08:58:59 +0000 Subject: [PATCH 3/5] [SPARK-59685][SQL] Narrow routing comments and assert the JSON_EXISTS default mode Address review feedback: - The `RoutedSqlJsonExpression` doc and two JsonArraySuite comments overstated the contract: a clause-free JSON_QUERY / JSON_ARRAY that is a top-level JSON_ARRAY element is constructed directly by `AstBuilder` (not routed), and a spliced element (explicit or implicit FORMAT JSON) forces the direct grammar branch so its rendering carries no default RETURNING STRING (e.g. splicedOmit). Narrow the comments to when routing and the ownership clause actually apply. - The JSON_EXISTS round-trip test only evaluated valid input, whose Row(true) is identical across every ON ERROR mode, so it re-proved built-in binding but not the emitted default. Assert the rendered clause is FALSE ON ERROR so a renderer that changed the default is caught. Co-authored-by: Isaac --- .../sql/catalyst/expressions/jsonExpressions.scala | 9 +++++---- .../scala/org/apache/spark/sql/JsonArraySuite.scala | 11 ++++++----- .../scala/org/apache/spark/sql/JsonExistsSuite.scala | 5 +++++ 3 files changed, 16 insertions(+), 9 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala index c059db74d5069..3318337555627 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala @@ -689,10 +689,11 @@ object JsonValueBehavior { // scalastyle:on line.size.limit /** - * A SQL/JSON function whose clause-free call `AstBuilder` routes through function resolution, so - * its canonical `sql` must append the default clause to reparse back to this built-in instead of - * a same-named routine on the SQL PATH. `sqlString`'s `forceBuiltinOwnership` gates that clause - * (true for `sql`, false for the never-reparsed pretty form) and `renderChild` renders each child. + * A SQL/JSON built-in whose clause-free call can route through function resolution, letting a + * same-named routine on the SQL PATH shadow it. So when `sql` renders an otherwise clause-free + * form, it appends the default clause to keep the reparse bound to this built-in. `sqlString`'s + * `forceBuiltinOwnership` gates that clause (true for `sql`, false for the never-reparsed pretty + * form) and `renderChild` renders each child. */ trait RoutedSqlJsonExpression extends Expression { private[sql] def sqlString( diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala index 57fff7378bac3..f6de164248686 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonArraySuite.scala @@ -356,9 +356,9 @@ class JsonArraySuite extends QueryTest with SharedSparkSession { test("emitted SQL reparses and evaluates with raw-vs-quoted semantics preserved") { // The .sql renderings above are round-trip contracts: reparsing and evaluating the actual - // emitted SQL -- including the default RETURNING STRING clauses it now carries -- must - // reproduce the original splicing. A bare nested constructor stays spliced; a cast-neutralized - // one stays quoted. + // emitted SQL -- including any default RETURNING STRING clauses it carries -- must reproduce + // the original splicing. A bare nested constructor stays spliced; a cast-neutralized one stays + // quoted. val inner = JsonArray( Seq(Literal(1)), Seq(false), Seq(false), JsonConstructorNullBehavior.Absent, StringType) val spliced = JsonArray( @@ -419,8 +419,9 @@ class JsonArraySuite extends QueryTest with SharedSparkSession { assert( splicedOmit.sql == """JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a' OMIT QUOTES) FORMAT JSON)""") - // Each emitted rendering (with the default RETURNING STRING it now carries) must reparse and - // evaluate the same as its clause-free equivalent, i.e. the added clause is semantically inert. + // Each emitted rendering must reparse and evaluate the same as its clause-free equivalent, so + // any default RETURNING STRING it carries is semantically inert (splicedOmit, whose spliced + // element forces the direct grammar branch, carries none). checkAnswer(sql(s"SELECT ${splicedKeep.sql}"), sql("""SELECT JSON_ARRAY(JSON_QUERY('{"a":{"x":1}}', '$.a'))""").collect().toSeq) checkAnswer(sql(s"SELECT ${quotedKeep.sql}"), diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala index a43dece658a50..c49eb8a5f1b74 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala @@ -80,6 +80,11 @@ class JsonExistsSuite extends QueryTest with SharedSparkSession { assert(reparsed.queryExecution.analyzed.expressions .exists(_.exists(_.isInstanceOf[JsonExists])), s"canonical SQL bound the shadow instead of the built-in: ${jsonExists.sql}") + // Valid input returns true under every ON ERROR mode, so also assert the emitted clause is + // the default FALSE ON ERROR -- otherwise a renderer that changed the default would still + // bind to the built-in and pass this round-trip. + assert(jsonExists.sql.contains("FALSE ON ERROR"), + s"canonical SQL changed the default ON ERROR mode: ${jsonExists.sql}") checkAnswer(reparsed, Row(true)) } finally { sql("SET PATH = DEFAULT_PATH") From db46e9ed5730dd5a04aeb9cccb45c4be03b314e9 Mon Sep 17 00:00:00 2001 From: Ganesha S Date: Thu, 24 Sep 2026 16:35:44 +0000 Subject: [PATCH 4/5] [SPARK-59685][SQL] Attach JSON_VALUE Scaladoc and cover clause suppression Move the RoutedSqlJsonExpression trait so JSON_VALUE's Scaladoc precedes JsonValue again, and add render-and-reparse tests for the explicit-clause suppression paths in JsonValueSuite and JsonExistsSuite. Co-authored-by: Isaac --- .../expressions/jsonExpressions.scala | 26 +++++++++---------- .../apache/spark/sql/JsonExistsSuite.scala | 19 ++++++++++++++ .../org/apache/spark/sql/JsonValueSuite.scala | 24 +++++++++++++++++ 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala index 3318337555627..8ff780c7488d2 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala @@ -646,6 +646,19 @@ case class JsonTable( copy(child = newChild) } +/** + * A SQL/JSON built-in whose clause-free call can route through function resolution, letting a + * same-named routine on the SQL PATH shadow it. So when `sql` renders an otherwise clause-free + * form, it appends the default clause to keep the reparse bound to this built-in. `sqlString`'s + * `forceBuiltinOwnership` gates that clause (true for `sql`, false for the never-reparsed pretty + * form) and `renderChild` renders each child. + */ +trait RoutedSqlJsonExpression extends Expression { + private[sql] def sqlString( + forceBuiltinOwnership: Boolean, + renderChild: Expression => String): String +} + /** * Behavior of `JSON_VALUE`'s `ON EMPTY` / `ON ERROR` clause: what to produce when the path matches * nothing, or when the input/extraction fails. @@ -688,19 +701,6 @@ object JsonValueBehavior { */ // scalastyle:on line.size.limit -/** - * A SQL/JSON built-in whose clause-free call can route through function resolution, letting a - * same-named routine on the SQL PATH shadow it. So when `sql` renders an otherwise clause-free - * form, it appends the default clause to keep the reparse bound to this built-in. `sqlString`'s - * `forceBuiltinOwnership` gates that clause (true for `sql`, false for the never-reparsed pretty - * form) and `renderChild` renders each child. - */ -trait RoutedSqlJsonExpression extends Expression { - private[sql] def sqlString( - forceBuiltinOwnership: Boolean, - renderChild: Expression => String): String -} - case class JsonValue( child: Expression, path: String, diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala index c49eb8a5f1b74..08bffe3954ee8 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonExistsSuite.scala @@ -93,6 +93,25 @@ class JsonExistsSuite extends QueryTest with SharedSparkSession { } } + test("SPARK-59685: explicit-clause JSON_EXISTS canonical SQL keeps its clause and appends no " + + "default ON ERROR") { + // The ownership clause (FALSE ON ERROR) is appended only to an otherwise clause-free render, so + // an explicit ON ERROR must suppress it. Malformed input makes the mode observable: were the + // explicit TRUE ON ERROR dropped or replaced by the default, the round-trip result would flip + // from true to false. + val jsonExists = sql(s"SELECT json_exists('not json', '$$.a' TRUE ON ERROR)") + .queryExecution.analyzed.expressions + .flatMap(_.collect { case je: JsonExists => je }).head + val rendered = jsonExists.sql + assert(!rendered.contains("FALSE ON ERROR"), + s"canonical SQL appended a duplicate default clause: $rendered") + val reparsed = sql(s"SELECT $rendered") + assert(reparsed.queryExecution.analyzed.expressions + .exists(_.exists(_.isInstanceOf[JsonExists])), + s"canonical SQL did not reparse to the built-in: $rendered") + checkAnswer(reparsed, Row(true)) + } + test("SPARK-59685: a default JSON_EXISTS keeps a clean auto-generated column name") { val name = sql(s"SELECT json_exists('$doc', '$$.addr.city')").schema.head.name assert(!name.contains("ON ERROR"), s"column name leaked the ownership clause: $name") diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala index 8f91373f0b62f..60512f6a31665 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonValueSuite.scala @@ -88,6 +88,30 @@ class JsonValueSuite extends QueryTest with SharedSparkSession { } } + test("SPARK-59685: explicit-clause JSON_VALUE canonical SQL keeps its clause and appends no " + + "default ownership clause") { + // The ownership clause (RETURNING STRING) is appended only to an otherwise clause-free render, + // gated independently on RETURNING, ON EMPTY, and ON ERROR. Exercise one explicit clause at a + // time so a regression appending a duplicate or conflicting RETURNING STRING fails here. + Seq( + s"json_value('$doc', '$$.id' RETURNING INT)" -> Row(7), + s"json_value('$doc', '$$.missing' DEFAULT -1 ON EMPTY)" -> Row("-1"), + s"json_value('$doc', '$$.name' ERROR ON ERROR)" -> Row("Ada")).foreach { + case (query, expected) => + val jsonValue = sql(s"SELECT $query") + .queryExecution.analyzed.expressions + .flatMap(_.collect { case jv: JsonValue => jv }).head + val rendered = jsonValue.sql + assert(!rendered.contains("RETURNING STRING"), + s"canonical SQL appended a duplicate default clause: $rendered") + val reparsed = sql(s"SELECT $rendered") + assert(reparsed.queryExecution.analyzed.expressions + .exists(_.exists(_.isInstanceOf[JsonValue])), + s"canonical SQL did not reparse to the built-in: $rendered") + checkAnswer(reparsed, expected) + } + } + test("SPARK-59685: a default JSON_VALUE keeps a clean auto-generated column name") { // The canonical `sql` forces the default RETURNING STRING so it stays bound to the built-in on // reparse, but the display (column) name is never reparsed and must not leak that clause. From 464294547588d5c0f19adfe148a53c140a1761bd Mon Sep 17 00:00:00 2001 From: Ganesha S Date: Fri, 25 Sep 2026 05:44:10 +0000 Subject: [PATCH 5/5] [SPARK-59685][SQL] Round-trip explicit-clause JSON_QUERY canonical SQL Co-authored-by: Isaac --- .../org/apache/spark/sql/JsonQuerySuite.scala | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/sql/core/src/test/scala/org/apache/spark/sql/JsonQuerySuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/JsonQuerySuite.scala index 25dc0d7193941..af565f4a58b1e 100644 --- a/sql/core/src/test/scala/org/apache/spark/sql/JsonQuerySuite.scala +++ b/sql/core/src/test/scala/org/apache/spark/sql/JsonQuerySuite.scala @@ -293,6 +293,28 @@ class JsonQuerySuite extends QueryTest with SharedSparkSession { checkAnswer(sql(s"SELECT $rendered"), Row("""{"city":"NYC"}""")) } + test("SPARK-59685: explicit-clause canonical SQL round-trips, suppressing the ownership clause") { + // The default clause-free render appends `RETURNING STRING` for ownership; an explicit clause + // must suppress it (a spurious `RETURNING STRING` next to a clause would be invalid SQL). These + // cases reparse each mode's rendered `sql` to catch a renderer regression that the parse-only + // ExpressionParserSuite cannot -- e.g. a wrong keyword or a duplicated/conflicting clause. + Seq( + s"json_query('$doc', '$$.tags' WITH UNCONDITIONAL ARRAY WRAPPER)" -> Row("""[["x","y"]]"""), + s"json_query('$doc', '$$.id' WITH CONDITIONAL ARRAY WRAPPER)" -> Row("[7]"), + s"json_query('$doc', '$$.name' OMIT QUOTES)" -> Row("Ada"), + s"json_query('$doc', '$$.missing' EMPTY ARRAY ON EMPTY)" -> Row("[]"), + s"json_query('$doc', '$$.missing' EMPTY OBJECT ON EMPTY)" -> Row("{}"), + "json_query('not json', '$.a' EMPTY ARRAY ON ERROR)" -> Row("[]") + ).foreach { case (query, expected) => + val jsonQuery = sql(s"SELECT $query").queryExecution.analyzed.expressions + .flatMap(_.collect { case jq: JsonQuery => jq }).head + val rendered = jsonQuery.sql + assert(!rendered.contains("RETURNING STRING"), + s"explicit-clause render leaked the ownership clause: $rendered") + checkAnswer(sql(s"SELECT $rendered"), expected) + } + } + test("works over a column of JSON documents") { val df = Seq( """{"a":{"x":1}}""",