uros-b commented on code in PR #57395:
URL: https://github.com/apache/spark/pull/57395#discussion_r3715239104
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/ArrowCachedBatchSerializer.scala:
##########
@@ -170,9 +174,12 @@ class ArrowCachedBatchSerializer extends
SimpleMetricsCachedBatchSerializer {
val selectedSchema = DataTypeUtils.fromAttributes(selectedAttributes)
val timeZoneId = conf.sessionLocalTimeZone
- // Calculate column indices for projection
+ // Calculate column indices for projection. Use AttributeSeq's cached
exprId -> ordinal
+ // map so each selected attribute is a constant-time lookup, instead of
linear-scanning
+ // the cache attributes per selected attribute.
+ val cacheAttributeSeq = AttributeSeq(cacheAttributes)
val selectedIndices = selectedAttributes.map { attr =>
Review Comment:
selectedIndices is computed here, before the needsFallback check at line
203, but the fallback branch never uses it: it delegates to
convertCachedBatchToColumnarBatch, which builds its own AttributeSeq and
recomputes the same array. Only the else branch at line 229 consumes
selectedIndices. The dead computation predates this PR, but the PR is rewriting
these exact lines, and after the change it costs a second map build instead of
a second scan, so it seems worth moving the val into the else branch alongside
prefetchEnabled while you are here.
--
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]
