LuciferYang opened a new pull request, #57768:
URL: https://github.com/apache/spark/pull/57768
### What changes were proposed in this pull request?
Follow-up to #56430, addressing review comments from cloud-fan.
1. The JDK backend rewrote a reference to a class Java cannot name - an
anonymous or
local class, or a class nested inside one - into its nearest nameable
supertype
unconditionally. That is only sound when the supertype is itself
referenceable and
offers every member the generated code could access, and nothing enforced
either
condition. Such a unit is now routed to Janino instead, joining the two
existing
deterministic-routing cases (REPL contexts and Scala package-object
classes).
`canNarrowSafely` decides this. A member matches by its exact erased
signature, with
an allowance for bridges: an override of a generic method erases narrower
than the
supertype declaration it implements (`compare(String, String)` against
`Comparator.compare(Object, Object)`) and carries a bridge with the
supertype's
signature, so narrowing keeps dispatching to the override. An overload
has no bridge
and is rejected - it must be, because `Invoke` codegen wraps every call
in an explicit
cast, which would hide the resulting type mismatch from javac and
silently bind the
call to the supertype's method. The replacement type and all its
enclosing classes
must also be public: same-package is not sufficient, because the
generated class is
loaded by `InMemoryClassLoader` and its runtime package differs from the
same-named
package on the application loader.
`nameableSupertype` now climbs while `getCanonicalName` is null rather
than while the
class is anonymous or local, which additionally covers a named class
nested inside
one of those - neither anonymous nor local itself, yet equally unnameable.
2. Reflection in this path can raise a `LinkageError` when a class loads but
a type in
its signature does not (a partial or shaded jar). `NonFatal` does not
cover that and
an escaping `Error` would bypass the codegen fallbacks, so
`canNarrowSafely` and
`sourceNameOf` catch it and degrade to "cannot narrow" / the binary name.
3. Comment and documentation fixes: two wordings in the
`CodeCompiler.active` scaladoc,
and the `spark.sql.codegen.compiler` config doc, which enumerated only two
always-Janino cases.
### Why are the changes needed?
Fixes the unchecked assumption. Without it, an anonymous type whose accessed
member is
absent from the supertype either fails to compile or - for a same-arity
overload -
compiles clean and returns the supertype's answer.
### Does this PR introduce _any_ user-facing change?
No. The default backend is Janino, and for `spark.sql.codegen.compiler=jdk`
this only
moves units that javac would have mishandled onto the working path.
### How was this patch tested?
Eight cases in `CodeCompilerSuite` covering the `$`-digit gate, narrowable
and
unnarrowable shapes, an unnameable supertype, a class nested in a local
class, the
routing decision under both backend settings, an end-to-end compile
asserting the JDK
backend rejects the unit that narrowing breaks, and its counterpart
asserting a
bridge-preserved call stays on the JDK backend. Each production check was
mutated in
turn and the corresponding test fails without it.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 5
--
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]
