ganeshashree commented on code in PR #57395:
URL: https://github.com/apache/spark/pull/57395#discussion_r3718239700
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/InMemoryRelation.scala:
##########
@@ -197,8 +197,12 @@ class DefaultCachedBatchSerializer extends
SimpleMetricsCachedBatchSerializer {
conf: SQLConf): RDD[ColumnarBatch] = {
val offHeapColumnVectorEnabled = conf.offHeapColumnVectorEnabled
val outputSchema = DataTypeUtils.fromAttributes(selectedAttributes)
+ // Use AttributeSeq's cached exprId -> ordinal map so each selected
attribute is a
+ // constant-time lookup, instead of rebuilding the exprId list and
linear-scanning it
+ // per selected attribute.
+ val cacheAttributeSeq = AttributeSeq(cacheAttributes)
val columnIndices =
- selectedAttributes.map(a => cacheAttributes.map(o =>
o.exprId).indexOf(a.exprId)).toArray
+ selectedAttributes.map(a => cacheAttributeSeq.indexOf(a.exprId)).toArray
Review Comment:
Done, extracted into CachedColumnIndices (new file in execution/columnar),
and all four call sites now go through it: `InMemoryRelation.scala:200` and
`:233`, `ArrowCachedBatchSerializer.scala:147` and `:213`.
As you suggested, the row path in `DefaultCachedBatchSerializer` calls the
helper and derives the types from selectedAttributes rather than keeping its
own copy, so the unzip of parallel ordinal/dataType lists is gone.
One deviation from your suggested placement, and happy to change it if you
disagree: I put it in a `private[columnar]` object rather than on the
`CachedBatchSerializer` trait. The trait is `@DeveloperApi`, so a method there
becomes public API surface needing a `@Since` and a version decision , that
turns this cleanup into an API change. `private[columnar]` also matches the
existing convention in this package (`ColumnBuilder, ColumnType,
ByteBufferHelper`). If the intent is for third-party serializers outside
`execution.columnar` to be able to call it, then it does need to be on the
trait, but I'd rather do that deliberately than as a side effect of
deduplication.
--
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]
