uros-b commented on code in PR #56621:
URL: https://github.com/apache/spark/pull/56621#discussion_r3461230843
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/statsEstimation/SizeInBytesOnlyStatsPlanVisitor.scala:
##########
@@ -55,7 +67,8 @@ object SizeInBytesOnlyStatsPlanVisitor extends
LogicalPlanVisitor[Statistics] {
override def default(p: LogicalPlan): Statistics = p match {
case p: LeafNode => p.computeStats()
case _: LogicalPlan =>
- Statistics(sizeInBytes = p.children.map(_.stats.sizeInBytes).filter(_ >
0L).product)
+ val sizeInBytes = p.children.map(_.stats.sizeInBytes).filter(_ >
0L).product
+ cappedStats(sizeInBytes)
Review Comment:
Right now the cap is applied after .product. That is safe once the fix is in
place end-to-end, but leaf nodes still bypass the visitor cap via
computeStats(). If a leaf ever surfaced with an uncapped astronomical
sizeInBytes (e.g. poisoned originStats from an older Spark version),
multiplication could still throw before cappedStats runs. Defensive hardening:
```
val sizeInBytes = p.children
.map(_.stats.sizeInBytes.min(MAX_SIZE_IN_BYTES))
.filter(_ > 0L)
.product
cappedStats(sizeInBytes)
```
--
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]
