uros-b commented on code in PR #57395:
URL: https://github.com/apache/spark/pull/57395#discussion_r3715235336
##########
sql/core/src/main/scala/org/apache/spark/sql/execution/columnar/ArrowCachedBatchSerializer.scala:
##########
@@ -144,8 +144,12 @@ class ArrowCachedBatchSerializer extends
SimpleMetricsCachedBatchSerializer {
conf: SQLConf): RDD[ColumnarBatch] = {
val cacheSchema = DataTypeUtils.fromAttributes(cacheAttributes)
val selectedSchema = 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.
Review Comment:
These three lines describe what the previous code did and why the new code
is faster, which is PR-description material and stops being useful once this
merges.
The genuinely non-obvious thing at this call site is the opposite: why the
explicit AttributeSeq(...) wrapper rather than the implicit conversion. If
someone "simplifies" this to cacheAttributes.indexOf(a.exprId), the inherited
Seq.indexOf[B >: A](elem: B) wins overload resolution with B inferred as Any,
the implicit never fires, and every column silently resolves to -1. Worth to
prevent that:
```
// AttributeSeq wrapper is required: Seq.indexOf would resolve against the
element type
// (B >: Attribute inferred as Any) and silently return -1 for every column.
```
Same for the copies at ArrowCachedBatchSerializer.scala:177-179,
InMemoryRelation.scala:200-202 and InMemoryRelation.scala:237-239. If you take
the helper suggestion, this comment lives on the helper only.
--
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]
