Skip to content

Commit 58757cb

Browse files
committed
update shading comments after #4325
1 parent 79a4e98 commit 58757cb

3 files changed

Lines changed: 12 additions & 13 deletions

File tree

spark/src/main/scala/org/apache/comet/codegen/CometBatchKernelCodegen.scala

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,8 @@ import org.apache.comet.shims.CometExprTraitShim
3535
* fuses Arrow input reads, Spark expression evaluation, and Arrow output writes into one
3636
* Janino-compiled method per `(expression, schema)` pair.
3737
*
38-
* The kernel is generic over Catalyst expressions and does not assume the bound tree came from a
39-
* `ScalaUDF`. Today's only consumer is [[org.apache.comet.udf.codegen.CometScalaUDFCodegen]].
38+
* The kernel compiles any bound Catalyst expression; the tree need not be rooted at a `ScalaUDF`.
39+
* Today's only consumer is [[org.apache.comet.udf.codegen.CometScalaUDFCodegen]].
4040
*
4141
* Constraints: one output vector per kernel; per-row scalar evaluation only (aggregate, window,
4242
* generator are rejected by [[canHandle]]).
@@ -52,11 +52,10 @@ import org.apache.comet.shims.CometExprTraitShim
5252
object CometBatchKernelCodegen extends Logging with CometExprTraitShim {
5353

5454
/**
55-
* Resolve an Arrow vector class by simple name through the same classloader the codegen uses
56-
* internally. The `common` module shades `org.apache.arrow` to `org.apache.comet.shaded.arrow`,
57-
* so `classOf[VarCharVector]` at a call site in an unshaded module refers to a different
58-
* [[Class]] object than the one the codegen pattern-matches against. Tests resolve through
59-
* this.
55+
* Resolve an Arrow vector class by simple name through the codegen object's own classloader.
56+
* Tests use this to refer to vector classes via the same classloader the codegen pattern-
57+
* matches against, in case the test classpath ever diverges from the codegen's (e.g. through
58+
* future shading rearrangement).
6059
*/
6160
def vectorClassBySimpleName(name: String): Class[_ <: ValueVector] = name match {
6261
case "BitVector" => classOf[BitVector]
@@ -234,8 +233,10 @@ object CometBatchKernelCodegen extends Logging with CometExprTraitShim {
234233
ctx.INPUT_ROW = "row"
235234

236235
val baseClass = classOf[CometBatchKernel].getName
237-
// Resolve shaded Arrow class names so generated source matches the abstract method signature
238-
// after Maven relocation.
236+
// Resolve Arrow class names at runtime so the generated source matches the method signature
237+
// the running classloader sees. The packaged Comet jar relocates `org.apache.arrow` to
238+
// `org.apache.comet.shaded.arrow` (see `spark/pom.xml`); `.getName` picks the right name
239+
// regardless of whether we run against the shaded jar or the unshaded build output.
239240
val valueVectorClass = classOf[ValueVector].getName
240241
val fieldVectorClass = classOf[FieldVector].getName
241242

spark/src/test/scala/org/apache/comet/CometCodegenAssertions.scala

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ trait CometCodegenAssertions {
6464
/**
6565
* Asserts a kernel matching the given input Arrow vector classes and output type sits in the
6666
* JVM-wide signature set. Pair with `assertCodegenRan` since the set is append-only. Compares
67-
* by simple name because `common` shades `org.apache.arrow`.
67+
* by simple name to be robust to Arrow shading.
6868
*/
6969
protected def assertKernelSignaturePresent(
7070
inputs: Seq[Class[_ <: ValueVector]],

spark/src/test/scala/org/apache/comet/CometCodegenSourceSuite.scala

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,9 +31,7 @@ import org.apache.comet.codegen.CometBatchKernelCodegen.{ArrayColumnSpec, ArrowC
3131
import org.apache.comet.udf.codegen.CometScalaUDFCodegen
3232

3333
// Resolve Arrow vector classes through the codegen object so tests see the same `Class` objects
34-
// the shaded `common` module sees. A direct `classOf[org.apache.arrow.vector.VarCharVector]` here
35-
// would be the unshaded class from the test classpath, which is not `==` to the shaded class the
36-
// production pattern-matches against.
34+
// the codegen pattern-matches against, regardless of any future shading rearrangement.
3735

3836
/**
3937
* Generated-source inspection tests. These exercise `CometBatchKernelCodegen.generateSource` and

0 commit comments

Comments
 (0)