ganeshashree commented on PR #57395: URL: https://github.com/apache/spark/pull/57395#issuecomment-5033797453
> On another note, PTAL at ArrowCachedBatchSerializer.scala:148 - the sibling serializer's convertCachedBatchToColumnarBatch still carries the identical O(n·m) pattern (selectedAttributes.map(a => cacheAttributes.map(o => o.exprId).indexOf(a.exprId)).toArray) that this PR removes from DefaultCachedBatchSerializer. > > Since the stated goal is removing the quadratic column-index computation, leaving it in the Arrow fast path (the serializer used when Arrow caching is enabled, [SPARK-57268](https://issues.apache.org/jira/browse/SPARK-57268), and at least as relevant for wide tables) makes the cleanup incomplete. The fix is a direct copy of the same map lookup. ArrowCachedBatchSerializer.convertCachedBatchToColumnarBatch now uses AttributeSeq.indexOf as well, so the quadratic pattern is gone from the Arrow fast path too. While there, I noticed the sibling convertCachedBatchToInternalRow in the same serializer had the same defect in a slightly different form: `cacheAttributes.indexWhere(_.exprId == attr.exprId)`, so I converted that one too. Both Arrow paths now go through the shared AttributeSeq map lookup, matching DefaultCachedBatchSerializer. Verified with ArrowCachedBatchSerializerSuite (covering both the columnar and row paths, including prefetch), all green. -- 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]
