LuciferYang opened a new pull request, #57723:
URL: https://github.com/apache/spark/pull/57723
### What changes were proposed in this pull request?
`ColumnarIterator` now extends
`scala.collection.AbstractIterator[InternalRow]` instead of mixing in
`Iterator[InternalRow]` directly. `AbstractIterator` is the standard library's
`abstract class AbstractIterator[+A] extends Iterator[A]`, documented as
"explicit instantiation of the `Iterator` trait to reduce class file size in
subclasses".
The PR also drops a dead `import
org.apache.spark.sql.catalyst.expressions.codegen.BufferHolder;` from the
generated source template. Nothing in the template has referenced
`BufferHolder` since a7c19d9c21d (SPARK-23713, 2018), which moved the generated
writes to `UnsafeRowWriter` and made `BufferHolder` package-private in the same
commit.
### Why are the changes needed?
Mixing a trait into a class that fixes its type parameter makes scalac copy
the trait's forwarders into that class file, and emit them raw with no
`Signature` attribute. Two consequences:
1. The class file is much larger than it needs to be.
`ColumnarIterator.class` goes from 31,491 to 1,484 bytes here, because the
forwarders stay in `AbstractIterator` rather than being duplicated.
2. javac cannot subclass it. A minimal reproduction, with scalac 2.13.18 and
no Spark involved:
```scala
abstract class DirectIter extends Iterator[String] { def initialize(): Unit }
abstract class AbsIter extends scala.collection.AbstractIterator[String] {
def initialize(): Unit }
```
```java
public class PA extends DirectIter { /* hasNext, next, initialize */ }
public class PB extends AbsIter { /* hasNext, next, initialize */ }
```
```
$ javac PA.java
PA.java:1: error: minBy(Function1,Ordering) in DirectIter cannot implement
<B>minBy(Function1<A,B>,Ordering<B>) in IterableOnceOps
return type Object is not compatible with String
$ javac PB.java # no error
```
`AbstractIterator` keeps its element type generic, so its forwarders retain
their signatures and no clash arises.
The second point is what makes this a prerequisite for SPARK-57403: the
`SpecificColumnarIterator` subclass that `GenerateColumnAccessor` emits is
compiled as Java source, so under the javac backend it hits exactly the error
above. Janino does not perform this check, which is why the direct form has
worked so far.
### Does this PR introduce _any_ user-facing change?
No. `AbstractIterator[+A] extends Iterator[A]`, so `ColumnarIterator`
remains a `scala.collection.Iterator[InternalRow]` and its public API is
unchanged.
### How was this patch tested?
Existing tests: `sql/testOnly org.apache.spark.sql.execution.columnar.*
org.apache.spark.sql.CachedTableSuite org.apache.spark.sql.DatasetCacheSuite`.
`InMemoryColumnarQuerySuite`'s "SPARK-14138: Generated SpecificColumnarIterator
can exceed JVM size limit for cached DF" exercises the generated subclass.
No new test is added. What this change protects against is javac rejecting
the generated subclass, and the javac backend is not in the tree yet; asserting
on the superclass of `ColumnarIterator` would only restate the diff. The
class-file size is not something the build asserts on either.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code 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]
