Yang Jie created SPARK-58437:
--------------------------------
Summary: Fix invalid Java constructs in generated code that Janino
tolerates
Key: SPARK-58437
URL: https://issues.apache.org/jira/browse/SPARK-58437
Project: Spark
Issue Type: Sub-task
Components: SQL
Affects Versions: 5.0.0
Reporter: Yang Jie
Several codegen templates emit Java source that is not valid Java. Janino
accepts it
because it erases generics and does not enforce all of the rules javac does, so
these have
gone unnoticed. One of them has a real runtime consequence independent of which
compiler
compiles the generated code.
*1. A non-String value is stored into a {{Map<String, String>}}*
{{Sequence.genSequenceLengthCode}} builds the message parameters for
{{_LEGACY_ERROR_TEMP_3243}}:
{code:java}
java.util.Map<String, String> params = new java.util.HashMap<String, String>();
params.put("start", $start); // $start is a numeric expression
params.put("stop", $stop);
params.put("step", $step); // may be a CalendarInterval reference
{code}
Janino erases the type arguments and binds these to {{put(Object, Object)}}, so
numeric
values are autoboxed and, at the {{InternalSequenceBase}} call site, {{$step}}
is an
interval object. The map ends up holding {{java.lang.Long}} /
{{CalendarInterval}} values.
It reaches {{SparkIllegalArgumentException}} and is handed back by
{{SparkThrowable.getMessageParameters()}}, whose declared type is
{{java.util.Map<String, String>}} - a caller reading a value as {{String}} gets
a
{{ClassCastException}}. The rendered message text happens to be correct because
substitution calls {{toString}}. This error path currently has no test coverage.
*2. A {{final}} local variable is reassigned*
{{Sequence.doGenCode}} declares {{final $arrElemType[] $arr = null;}} and then
passes
{{$arr}} to {{impl.genCode}}, which assigns to it. Assigning to a {{final}}
local is
illegal Java; Janino does not enforce it.
*3. Binary inner-class names in source position*
{{ArrayDistinct}}, {{ArrayUnion}}, {{ArrayIntersect}} and {{ArrayExcept}} emit
{{scala.collection.mutable.ArrayBuilder$ofInt}}. {{$}} is the JVM binary name
separator,
not source syntax; the source form is {{ArrayBuilder.ofInt}}. Janino resolves
the binary
name directly.
*4. A field declared with a type argument that does not match what is assigned*
{{SortExec.doProduce}} declares the mutable state as
{{scala.collection.Iterator<UnsafeRow>}} and assigns
{{UnsafeExternalRowSorter.sort()}} to
it, which returns {{Iterator<InternalRow>}}. The rows are in fact {{UnsafeRow}}
- the
consumer casts each one back - so this is not a runtime defect, but the
declared type
argument is wrong.
*5. Generic array creation*
{{CodegenContext.declareMutableStates}} compacts same-typed mutable states into
an array,
emitting {{new Foo<X>[n]}} when the type is parameterized. Java forbids generic
array
creation; the idiom is {{Foo<X>[] a = new Foo[n]}}. Reachable today through
{{SampleExec.doConsume}} without replacement, whose
{{BernoulliCellSampler<UnsafeRow>}}
state is not force-inlined and therefore takes the array-compaction path.
Fixes 2-5 are no-ops under Janino; fix 1 corrects the parameter map's value
type. All five
are also prerequisites for compiling generated code with a stricter Java
compiler
(SPARK-57370).
Out of scope, noted here for the record: for the same condition, the
interpreted path
({{Sequence.getSequenceLength}}) throws a plain {{IllegalArgumentException}} via
{{require}} with no error class, while the codegen path throws a
{{SparkIllegalArgumentException}} carrying {{_LEGACY_ERROR_TEMP_3243}}.
Reconciling the
two would change a user-visible exception type and belongs in its own change.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
