cloud-fan commented on code in PR #59828:
URL: https://github.com/apache/spark/pull/59828#discussion_r4434559154
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -1314,6 +1311,46 @@ trait ImplicitlyFormattedAsJson extends Expression {
def emitsImplicitJsonText: Boolean = true
}
+object ImplicitlyFormattedAsJson {
+ /**
+ * Whether `e` emits JSON text, so a JSON constructor splices it instead of
quoting it. Sees
+ * through `Collate`, which only changes the collation, but not `Cast`:
`CAST(... AS STRING)` is
+ * how a user (and `JsonArray.sql`) asks for the value to be quoted.
+ */
+ @scala.annotation.tailrec
+ def isImplicitlyJson(e: Expression): Boolean = e match {
+ case c: Collate => isImplicitlyJson(c.child)
+ case i: ImplicitlyFormattedAsJson => i.emitsImplicitJsonText
+ case _ => false
+ }
+
+ /**
+ * The splice decision for a resolved argument, used by the builders and by
the parser wrappers
+ * when they finalize their flags. Like [[isImplicitlyJson]], but also sees
through the collation
+ * `Cast` that the analyzer adds under an object or view default collation
(mixed with `Collate`
+ * in any order). A user cast, including `string(...)`, still makes the
value quoted.
+ */
+ @scala.annotation.tailrec
+ def splicesThroughCollationCast(e: Expression): Boolean = e match {
Review Comment:
**Nit (P3):** For `SELECT json_array(x) AS a FROM VALUES (1) AS t(x) GROUP
BY x, json_array(a)` (GROUP BY aliases are on by default), the fixed-point
analyzer substitutes the select-list `Alias(json_array(x), a)` into the
argument and trims it only after the grouping expression resolves. This
predicate does not look through an `Alias`, so the builder freezes `formatJson
= [false]` and produces `["[1]"]`. The single-pass resolver substitutes the
alias child, so it builds `formatJson = [true]` and produces `[[1]]`. The same
split applies to `json_object('k', a)`, `builtin.json_array(a)` and
`json_array(a NULL ON NULL)`.
The two analyzers therefore build different grouping keys for the same
query: dual-run analysis in tests reports a plan mismatch, and the key differs
whenever the single-pass resolver is enabled. Default fixed-point analysis
alone is consistent, and I found no query whose output rows differ. On the base
branch both analyzers quoted.
**Recommended change:** Let the shared splice predicate see through an
Alias, so a GROUP BY alias that names a built-in JSON producer is spliced in
fixed-point as it already is in single-pass, matching the declared rule that an
argument resolving to a built-in producer is spliced. List the Alias unwrap in
the predicate's scaladoc, and add GROUP BY alias cases for both constructors
that run under dual-run analysis.
**Why this works:** With the Alias unwrapped, the fixed-point builder or
stock wrapper sees the same producer the single-pass resolver passes, so both
analyzers set the same splice flag and build the same grouping key.
**Scope:** Teach the shared splice predicate to look through an alias,
document it, and cover GROUP BY aliases in tests.
**Compatibility:** User casts, string(...) and Collate keep their current
splice decisions, and arguments that are not aliases splice or quote exactly as
before.
**Risks:** Every argument that reaches the predicate as an Alias over a
built-in JSON producer now splices, not only a GROUP BY alias. For example, a
DataFrame or Connect call such as call_function("json_array",
expr("json_array(1)").as("x")) is quoted on the base branch and with this PR,
and becomes [[1]], which follows the declared rule for an argument that
resolves to a built-in producer. Other routes that pass an alias to a
constructor argument were not enumerated. Removing the Alias case restores the
current results.
**Constraints:** Unwrap only an Alias and keep stopping at a user cast, so a
user CAST around an alias reference still quotes. Keep the Collate and analyzer
collation cast handling unchanged.
**Success:** For a GROUP BY expression such as json_array(a),
json_object('k', a), json_array(a NULL ON NULL) or builtin.json_array(a), where
a is a GROUP BY alias of a JSON producer, both analyzers build the same
grouping key with the argument spliced. A user cast around such an alias
reference still keeps the value quoted. The splice predicate's scaladoc lists
every shape it accepts, including the Alias unwrap.
**Verification:**
- **Regression:** GROUP BY expressions that pass a GROUP BY alias of a JSON
producer to JSON_ARRAY and JSON_OBJECT, in flat, qualified and clause-bearing
forms, produce matching fixed-point and single-pass plans and the same rows.
- **Compatibility:** A user CAST(... AS STRING) around such an alias
reference still quotes the value in both analyzers.
- **Behavior:** A DataFrame call_function json_array or json_object call
that passes an aliased nested constructor splices it, with the same result in
both analyzers.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala:
##########
@@ -941,11 +941,10 @@ object FunctionRegistry {
expression[StructsToCsv]("to_csv")
)
- // Built-in forms of the SQL:2016 JSON constructor and path functions.
`AstBuilder` routes the
- // eligible flat clause-free calls here so a same-named routine can shadow
them. Clause-bearing
- // forms, and the nested/implicit-JSON clause-free forms (routing deferred
to SPARK-59243), are
- // still built directly by the grammar -- except the `json_object` comma
form, whose nested
- // producer value also routes here (carrying a `JsonImplicitFormatCarrier`).
+ // Built-in forms of the SQL:2016 JSON constructor and path functions.
`AstBuilder` routes
+ // clause-free calls here so a same-named routine can shadow them; a
nested-constructor call
+ // routes as a `RoutedJsonConstructor`. Calls with a clause or explicit
FORMAT JSON, and calls
+ // written directly as another constructor's value, always mean the built-in.
Review Comment:
**Nit (P3):** This sentence covers the whole routed list below, but only
`visitJsonQuery`, `visitJsonArray` and `visitJsonObject` check
`isDirectJsonConstructorArgument`. `visitJsonValue` and `visitJsonExists` route
every clause-free call wherever it is written, and the shadow test in
`JsonArraySuite` asserts that `json_array(json_value('{"a":"x"}', '$.a') NULL
ON NULL)` returns `["shadowed"]`. A reader working out shadow ownership from
this comment would conclude the opposite. Consider limiting the direct-value
part to JSON_ARRAY, JSON_OBJECT and JSON_QUERY, or saying that JSON_VALUE and
JSON_EXISTS values still route.
This extends an [existing
discussion](https://github.com/apache/spark/pull/59828#discussion_r4421746464).
**Verification:**
- **Inspection:** The comment's always-built-in list matches the parser:
calls with a clause or explicit FORMAT JSON, and JSON_ARRAY, JSON_OBJECT and
JSON_QUERY written directly as a constructor value.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -4501,7 +4497,7 @@ class AstBuilder extends DataTypeAstBuilder
// JSON_OBJECT defaults to NULL ON NULL per the standard (emits keys
with null values).
val nullBehavior = Option(body.nullBehavior)
.map(buildJsonConstructorNullBehavior).getOrElse(JsonConstructorNullBehavior.Null)
- JsonObjectExpr(members, rawJson, needsValidation, nullBehavior,
returning)
+ UnresolvedJsonObject(members, rawJson, needsValidation, nullBehavior,
returning)
Review Comment:
**Nit (P3):** The comment above `formatArgs` still says "Freeze each value's
splice decision from the lexical argument so a later optimizer rewrite that
swaps the child cannot change it". The flags passed here now feed
`StockJsonConstructor.finalizeFlags`, which adds a splice for each resolved
JSON producer value, for example the `'c' VALUE builtin.json_object('d', 2)`
member in `JsonObjectSuite`. The `visitJsonArray` comments now say that the
parser's flags are a lower bound that analysis can extend; consider aligning
this one with them.
This extends an [existing
discussion](https://github.com/apache/spark/pull/59828#discussion_r4421746441).
**Verification:**
- **Inspection:** Both parser comments describe parser flags as a lower
bound that analysis can extend, matching the stock wrapper's flag finalization.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -2594,18 +2620,44 @@ class Analyzer(
case q: LogicalPlan =>
q.transformExpressionsUpWithPruning(
- _.containsAnyPattern(UNRESOLVED_FUNCTION, GENERATOR),
+ _.containsAnyPattern(UNRESOLVED_FUNCTION, GENERATOR,
UNRESOLVED_JSON_CONSTRUCTOR),
ruleId) {
- case u @ UnresolvedFunction(nameParts, arguments, _, _, _, _, _, _)
- if functionResolution.hasLambdaAndResolvedArguments(arguments)
=> withPosition(u) {
- functionResolution.resolveFunction(u) match {
- case func: HigherOrderFunction => func
- case other => other.failAnalysis(
- errorClass =
"INVALID_LAMBDA_FUNCTION_CALL.NON_HIGHER_ORDER_FUNCTION",
- messageParameters = Map(
- "class" -> other.getClass.getCanonicalName))
+ case u @ UnresolvedFunction(_, arguments, _, _, _, _, _, _)
+ if functionResolution.hasLambdaAndResolvedArguments(arguments) =>
+ resolveHigherOrderFunction(u)
+
+ // Handle JSON constructors in this same bottom-up pass, so nesting
them inside other
+ // calls does not cost extra analyzer iterations.
+ case j: UnresolvedJsonConstructor
+ if functionResolution.routesJsonConstructorToFunction(j) =>
+ // A routine owns the name: resolve like the equivalent flat call.
+ val u = j.toUnresolvedFunction
+ if (functionResolution.hasLambdaAndResolvedArguments(u.arguments))
{
+ resolveHigherOrderFunction(u)
+ } else if (!u.childrenResolved) {
+ u
+ } else {
+ functionResolution.resolveFunction(u)
+ }
+
+ case j: UnresolvedJsonConstructor =>
+ val directStars = j.arguments.collect { case s: Star => s }
+ if (directStars.nonEmpty) {
+ throw QueryCompilationErrors.invalidStarUsageError(
+ s"expression `${j.prettyName}`", directStars)
+ }
+ // The built-in owns it. Switch to the built-in form so the owner
is checked only once.
+ val stock = j match {
+ case r: RoutedJsonConstructor => r.toStock
+ case s: StockJsonConstructor => s
+ }
+ if (!stock.childrenResolved) {
Review Comment:
**Blocking (P1):** This arm is still the only place where the fixed-point
analyzer materializes a stock wrapper, and it runs before `ResolveSubquery`,
`ResolveLambdaVariables`, `ResolveTimeZone` and the type coercion rules. When a
child becomes resolved only in one of those later rules, the wrapper waits a
full iteration. On the base branch the parser built `JsonObjectExpr` directly,
and `ResolveTimeZone` completed it in the same iteration as its child.
For example, `SELECT JSON_OBJECT('d' VALUE date_format(ts, 'yyyy') NULL ON
NULL) AS j FROM t` reaches its fixed point after two Resolution iterations on
the base branch and after three with this change. The same holds for `WHERE
JSON_ARRAY(CAST(ts AS STRING) NULL ON NULL) = '...'` and for `transform(arr, x
-> JSON_ARRAY(x NULL ON NULL)) AS j`, none of which involves a qualified or
routed producer, and the extra iteration recurs on every read of a view with
such a body. Only fixed-point analysis time is affected; the single-pass
resolver and the executed plan are not.
The new late-resolving-child test cannot catch this: it uses un-aliased
select items, where `ResolveAliases` needed the extra iteration on the base
branch too, and its `concat(...)` baseline also waits in `ResolveFunctions`.
This extends an [existing
discussion](https://github.com/apache/spark/pull/59828#discussion_r4418287789).
**Recommended change:** Also materialize a stock JSON constructor wrapper at
the end of each Resolution iteration, after ResolveTimeZone, ResolveSubquery,
ResolveLambdaVariables and the type coercion rules, once all of its children
are resolved, using the same built-in construction, flag finalization and
session time zone as the ResolveFunctions path. Extend the JSON constructor
analyzer-iteration tests to explicitly aliased and WHERE-clause constructors
whose child resolves late.
**Why this works:** A wrapper skipped earlier in the iteration because a
child was still unresolved is completed after the rules that finish that child,
so the constructor becomes resolved in the same iteration as its children, as
the parser-built node did on the base branch, while the splice decision still
reads only resolved children.
**Scope:** Add a late materialization step for stock JSON constructor
wrappers to the fixed-point Resolution batch and extend the analyzer-iteration
coverage.
**Compatibility:** Splice decisions keep reading only resolved children, and
routed wrappers keep deciding their owner once during function resolution.
**Risks:** It adds one pattern-pruned pass per Resolution iteration for
plans that still contain a stock wrapper and does nothing for other plans;
removing the step restores the current behavior.
**Constraints:** Only stock wrappers qualify; a routed wrapper whose owner
is not yet decided must not be materialized by the new step. The step must use
the same flag finalization and session time zone as the existing path, so
splice, quoting and validation results do not change. The single-pass resolver
stays unchanged because it has no iterations.
**Exclusions:**
- Flat clause-free calls built by the registry builders without a time zone,
which wait for the time-zone rule exactly as on the base branch.
**Success:** A clause-bearing, explicit FORMAT JSON or direct-argument
JSON_ARRAY/JSON_OBJECT whose child resolves after function resolution in an
iteration is materialized in that same iteration. Explicitly aliased and
non-select-list uses of such constructors need no more Resolution iterations
than on the base branch. Splice, quoting and validation results are unchanged
in both analyzers.
**Verification:**
- **Regression:** In an explicitly aliased select item and in a WHERE-clause
predicate, a clause-bearing JSON_OBJECT and JSON_ARRAY whose child is
time-zone-aware, needs type coercion, is a scalar subquery, or is a lambda
variable take no more Resolution-batch iterations than the same query with the
constructor replaced by a cast of the same child.
- **Behavior:** Splice, quoting, star, lambda, subquery, grouping and
default-collation results of both constructors stay the same.
- **Compatibility:** Deeply nested and function-alternating constructors
still analyze under a low iteration limit, and un-aliased select items keep
their current iteration count.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -1720,6 +1740,145 @@ object JsonArray {
}
}
+/**
+ * A parsed `JSON_ARRAY(...)` / `JSON_OBJECT(...)` that keeps the parser's
splice flags until its
+ * children resolve; the resolved children can then add splices the parser
could not see (e.g. a
+ * qualified `builtin.json_array(...)` argument).
+ *
+ * The built-in owns a [[StockJsonConstructor]]. A [[RoutedJsonConstructor]]
can be owned by a
+ * same-named routine or an `injectFunction` replacement, and then resolves as
an ordinary call.
+ */
+sealed trait UnresolvedJsonConstructor extends Unevaluable with
ImplicitlyFormattedAsJson {
+ def routeThroughResolution: Boolean
+ /** The arguments a routine owning the call receives. */
+ def arguments: Seq[Expression] = children
+ /** The built-in expression, with the splice flags finalized. */
+ def toBuiltin: TimeZoneAwareExpression
+ /** The ordinary call this stands for 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 built-in owns: a call with a clause or
explicit
+ * `FORMAT JSON`, a call written directly as another constructor's value (the
inner `JSON_ARRAY(1)`
+ * in `JSON_ARRAY(JSON_ARRAY(1))`), or a routed call the built-in turned out
to own.
+ */
+sealed trait StockJsonConstructor
+ extends UnresolvedJsonConstructor
+ with DefaultStringProducingExpression {
+ def returning: DataType
+ final override def routeThroughResolution: Boolean = false
+ // Report the RETURNING type, so an explicit `RETURNING STRING COLLATE c` is
not recolored to the
+ // object or view default collation.
+ override def dataType: DataType = returning
+
+ // Keep every parser flag and add a splice for each resolved child that is a
JSON producer. Such a
+ // child 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(_ && !_))
+ }
+}
+
+/**
+ * A clause-free [[UnresolvedJsonConstructor]] whose owner is decided during
resolution. Not a
+ * `DefaultStringProducingExpression`: until the owner is known 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
+ /** The built-in-owned form with the clause-free defaults, once the built-in
owns the call. */
+ def toStock: StockJsonConstructor
+ override def toBuiltin: TimeZoneAwareExpression = toStock.toBuiltin
+}
+
+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"
+ 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 nested constructor argument. */
Review Comment:
**Nit (P3):** The parser builds this wrapper whenever an argument satisfies
`ImplicitlyFormattedAsJson.isImplicitlyJson`, which includes a KEEP QUOTES
`JSON_QUERY`: `json_array(json_query('{"a":1}', '$'))` routes through it
although its argument is not a constructor. The branch comment in
`visitJsonArray` ("A nested constructor argument: still routed") also reads as
if the argument routes, while it is the call that routes and the nested
argument stays with the built-in. Consider describing both as a clause-free
call with a nested JSON producer argument, as the `UnresolvedRoutedJsonObject`
doc does.
**Verification:**
- **Inspection:** Both comments describe the same condition as the parser
branch and match the routed JSON_OBJECT wrapper's wording.
##########
sql/core/src/test/scala/org/apache/spark/sql/analysis/resolver/ResolverGuardSuite.scala:
##########
@@ -143,17 +143,21 @@ class ResolverGuardSuite extends ResolverGuardSuiteBase {
test("SQL JSON constructor and path functions") {
checkResolverGuard("""SELECT JSON_ARRAY(1, 'x', true)""")
checkResolverGuard("""SELECT JSON_ARRAY('[1]' FORMAT JSON)""")
+ // Nested clause-free JSON_ARRAY routes as UnresolvedRoutedJsonArray
before resolution; the
+ // guard gives it the flat call's owner check and admits the unshadowed
built-in.
+ checkResolverGuard("""SELECT JSON_ARRAY(JSON_ARRAY(1))""")
checkResolverGuard("""SELECT JSON_VALUE('{"a":1}', '$.a')""")
checkResolverGuard("""SELECT JSON_VALUE('{"a":1}', '$.a' RETURNING INT)""")
checkResolverGuard("""SELECT JSON_QUERY('{"a":[1]}', '$.a')""")
checkResolverGuard("""SELECT JSON_QUERY('{"a":[1]}', '$.a' WITH ARRAY
WRAPPER)""")
checkResolverGuard("""SELECT JSON_EXISTS('{"a":1}', '$.a')""")
checkResolverGuard("""SELECT JSON_EXISTS('{"a":1}', '$.a' TRUE ON
ERROR)""")
- // A clause-free JSON_OBJECT routes as an UnresolvedFunction unless a
VALUE/colon member is raw;
- // such a call, or a clause-bearing one, is built directly as a
JsonObjectExpr. Assert the guard
- // admits that direct form (removing its allowlist arm would fail here).
+ // Clause-bearing and explicit FORMAT JSON forms parse to a stock-owned
UnresolvedJsonObject;
Review Comment:
**Nit (P3):** Before this change, `JSON_ARRAY('[1]' FORMAT JSON)` and the
`RETURNING` and `FORMAT JSON` `JSON_OBJECT` queries here parsed to `JsonArray`
and `JsonObjectExpr`, and the old comment noted that removing that allowlist
arm would fail the test. They now reach the `StockJsonConstructor` arm, and no
case in this suite reaches the `JsonArray` or `JsonObjectExpr` arm. Those arms
still matter: `checkOperator` walks every operator, so a plan built on an
analyzed query that contains a resolved constructor still needs them. Consider
adding a guard case over such a plan, so that removing either arm fails.
**Verification:**
- **Regression:** Removing either the JsonArray or the JsonObjectExpr
allowlist arm makes the new guard check fail, while the existing stock-wrapper
cases still pass.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -709,6 +709,7 @@ class Analyzer(
ResolveAliases ::
ResolveSubquery ::
ResolveSubqueryColumnAliases ::
+ ResolveJsonConstructors ::
Review Comment:
Thanks, moving this into `ResolveFunctions` fixes the depth scaling and the
un-aliased select-list case. One shape still needs one more Resolution
iteration than the base branch: a clause-bearing constructor whose child
resolves later in the same iteration, when it is explicitly aliased or outside
the select list. This review raises it separately.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4418287789","thread_id":"inline:4418287789","verdict_sha256":"45cc9ffd5b74fb8a2016514ecfb13ff624805cb3c0e41ac0bd1b802d5e7d806d"}
-->
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala:
##########
@@ -1314,6 +1311,46 @@ trait ImplicitlyFormattedAsJson extends Expression {
def emitsImplicitJsonText: Boolean = true
}
+object ImplicitlyFormattedAsJson {
+ /**
+ * Whether `e` emits JSON text, so a JSON constructor splices it instead of
quoting it. Sees
+ * through `Collate`, which only changes the collation, but not `Cast`:
`CAST(... AS STRING)` is
+ * how a user (and `JsonArray.sql`) asks for the value to be quoted.
+ */
+ @scala.annotation.tailrec
+ def isImplicitlyJson(e: Expression): Boolean = e match {
+ case c: Collate => isImplicitlyJson(c.child)
+ case i: ImplicitlyFormattedAsJson => i.emitsImplicitJsonText
+ case _ => false
+ }
+
+ /**
+ * The splice decision for a resolved argument, used by the builders and by
the parser wrappers
+ * when they finalize their flags. Like [[isImplicitlyJson]], but also sees
through the collation
+ * `Cast` that the analyzer adds under an object or view default collation
(mixed with `Collate`
+ * in any order). A user cast, including `string(...)`, still makes the
value quoted.
Review Comment:
**Nit (P3):** `isAnalyzerCollationCast` only accepts a `Cast` whose direct
child is a `DefaultStringProducingExpression`, and `Collate` is not one, so
`Collate(Cast(producer))` splices while `Cast(Collate(producer))` is quoted.
The analyzers never build the second shape, so results are right, but a caller
relying on "in any order" for another shape would get a quoted value. Consider
stating the accepted shape instead: any number of `Collate` wrappers around at
most one analyzer collation cast placed directly on the producer.
**Verification:**
- **Inspection:** The documented shapes match the predicate's recursion and
its analyzer-cast check.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -4391,25 +4391,32 @@ class AstBuilder extends DataTypeAstBuilder
*/
override def visitJsonArray(ctx: JsonArrayContext): Expression =
withOrigin(ctx) {
val arrayValues = ctx.values.asScala.map(v => expression(v.value)).toSeq
- // Route a flat, clause-free call through routine resolution so it can be
shadowed. Cheap
- // clause/nesting checks come first to skip the recursive per-value FORMAT
scans when a clause
- // already forces direct construction.
- val routeThroughResolution =
+ // A clause-free call (no RETURNING, no ON NULL, not a direct JSON
constructor argument, and no
+ // explicit FORMAT JSON on any value) routes through routine resolution so
a routine can shadow
+ // it. The cheap clause/nesting checks come first so the recursive
per-value implicit-JSON scan
+ // runs only when routing is still possible.
+ val clauseFree =
ctx.returning == null && ctx.nullBehavior == null &&
!isDirectJsonConstructorArgument(ctx) &&
- !ctx.values.asScala.exists(_.FORMAT() != null) &&
- !arrayValues.exists(JsonArray.isImplicitlyJson)
- if (routeThroughResolution) {
+ !ctx.values.asScala.exists(_.FORMAT() != null)
+ if (clauseFree &&
!arrayValues.exists(ImplicitlyFormattedAsJson.isImplicitlyJson)) {
UnresolvedFunction("json_array", arrayValues, isDistinct = false)
+ } else if (clauseFree) {
+ // Nested clause-free call: route so a routine can shadow it, but carry
the per-value splice
+ // decision, frozen now from the lexical argument, through resolution
(see
Review Comment:
Thanks, the JSON_ARRAY comments read correctly now. The matching JSON_OBJECT
parser comment still says the splice decision is frozen from the lexical
argument; this review raises it separately.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4421746441","thread_id":"inline:4421746441","verdict_sha256":"45cc9ffd5b74fb8a2016514ecfb13ff624805cb3c0e41ac0bd1b802d5e7d806d"}
-->
##########
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:
Thanks, the stock-wrapper doc now covers direct constructor values. The new
registry comment goes further than the parser for JSON_VALUE and JSON_EXISTS;
this review raises it separately.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"inline:4421746464","thread_id":"inline:4421746464","verdict_sha256":"45cc9ffd5b74fb8a2016514ecfb13ff624805cb3c0e41ac0bd1b802d5e7d806d"}
-->
--
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]
