ganeshashree commented on code in PR #58005:
URL: https://github.com/apache/spark/pull/58005#discussion_r3900242316
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -4268,6 +4268,60 @@ class AstBuilder extends DataTypeAstBuilder
JsonQuery(jsonExpr, path, returning, wrapper, quotes, onEmpty, onError)
}
+ /**
+ * Resolve a `jsonConstructorNullBehavior` clause (`NULL` / `ABSENT`) into a
+ * [[JsonConstructorNullBehavior]].
+ */
+ private def buildJsonConstructorNullBehavior(
+ ctx: JsonConstructorNullBehaviorContext): JsonConstructorNullBehavior =
+ ctx match {
+ case _: JsonConstructorNullBehaviorNullContext =>
+ JsonConstructorNullBehavior.Null
+ case _: JsonConstructorNullBehaviorAbsentContext =>
+ JsonConstructorNullBehavior.Absent
+ }
+
+ /**
+ * Create a [[JsonArray]] expression for the SQL:2016 `JSON_ARRAY`
constructor function.
+ * The `ON NULL` clause defaults to `ABSENT ON NULL` (drops NULL elements),
and RETURNING
+ * defaults to STRING.
+ */
+ override def visitJsonArray(ctx: JsonArrayContext): Expression =
withOrigin(ctx) {
Review Comment:
Thanks, good catch. I dug into it, and the behavior is real. `JSON_ARRAY`'s
grammar branch sits before `#functionCall`, so a plain `JSON_ARRAY(1)` is built
directly, and the same-named routine on the path is bypassed.
The catch is that this is exactly how the three already-merged constructors
in the same family behave. `JSON_VALUE, JSON_QUERY, and JSON_EXISTS` are all
placed before `#functionCall` and all bypass routine lookup in the same way, as
do `first`, `any_value`, and `last`. I confirmed it on the current build: with
`SET PATH = system.session, system.builtin`, and a temporary `json_value`
returning `999`, `json_value('{"a":1}', '$.a')` still returns `1`. Functions
like `substring`, `trim`, and `overlay` do route their plain calls through
resolution, but only because their `FROM/FOR/PLACING` syntax is lexically
distinct from a function call. `JSON_ARRAY(1)` has no such distinguishing
token, so matching the substring model would mean registering `JSON_ARRAY` as a
real built-in and restructuring the grammar, plus doing the same to the other
three constructors to keep the family consistent.
So there's a scoping question here. Should we preserve routine lookup across
all the impacted functions (the merged `JSON_VALUE/JSON_QUERY/JSON_EXISTS`, and
ideally the wider grammar-function set) for consistency, which I'd handle as a
separate PR, or just apply it to `JSON_ARRAY` in this PR and accept that it
diverges from its siblings for now? Happy to go either way, just want to pick
the direction with you before changing anything.
--
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]
