cloud-fan commented on code in PR #58938:
URL: https://github.com/apache/spark/pull/58938#discussion_r4080429583
##########
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))
Review Comment:
**Non-blocking (P2):** This valid-input Row(true) result is the same for
FALSE, TRUE, UNKNOWN, and ERROR ON ERROR, while the expression-type check only
proves built-in binding. Could this round-trip test also evaluate malformed
JSON (or otherwise assert the emitted mode) so it fails if the canonical
renderer changes from the default FALSE ON ERROR?
##########
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
Review Comment:
**Nit (P3):** This contract is conditional: AstBuilder directly constructs
clause-free JsonQuery and JsonArray expressions when they are top-level
JSON_ARRAY elements, so not every implementation occurrence routes through
routine resolution. The related JsonArraySuite comment also says every asserted
rendering carries RETURNING STRING, but splicedOmit has no RETURNING clause,
and the clause is binding-relevant under a shadowing PATH. Could you narrow
both comments to the contexts and renderings where the ownership clause
actually applies?
--
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]
