LuciferYang opened a new issue, #12748:
URL: https://github.com/apache/gluten/issues/12748
### Backend
VL (Velox). The code is in `gluten-core` and applies to any backend that
registers transitions.
### Bug description
`TransitionCostModel#costComparator` breaks a cost tie on the transited
plan's node names, and does so by subtracting two `String` hashCodes:
```scala
nodeNames1.mkString.hashCode - nodeNames2.mkString.hashCode
```
That subtraction overflows. `"RowToVeloxColumnar"` hashes to 2056048280 and
`"CHColumnarToCarrierRow"` to -2037667767, so the true difference is
4093716047, past `Int.MaxValue`, and the int result wraps to -201251249. Across
the node names Gluten's transitions actually produce (`ColumnarToRow`,
`RowToColumnar`, `RowToVeloxColumnar`, `VeloxColumnarToRow`,
`ArrowColumnarToVeloxColumnar`, `VeloxColumnarToCarrierRow`, `LoadArrowData`,
`OffloadArrowData`, `RowToCHNativeColumnar`, `CHColumnarToRow`,
`CHColumnarToCarrierRow`), 46 of the 110 ordered pairs invert this way.
`FloydWarshallGraph#build` uses the comparator to decide whether a newly
found path replaces the incumbent, so an inverted sign can swap in an
equal-cost path that should have lost. The graph is built once per driver and
cached, so whichever path wins is then used for every query in that session.
### Gluten version
main (1.8.0-SNAPSHOT)
### Spark version
Version-agnostic (applies to spark-3.3 / 3.4 / 3.5 / 4.0 / 4.1).
### Spark configurations
None. The transition graph is built during session extension setup, with no
configuration needed to reach the comparator.
### System information
Not applicable. The affected code is
`gluten-core/src/main/scala/org/apache/gluten/extension/columnar/transition/TransitionGraph.scala`,
and does not depend on OS or hardware.
### Relevant logs
No exception. A wrong sign selects a different transition path silently, so
the only visible effect is the plan.
### Fix direction
Use `Integer.compare` on the two hash codes. That corrects the sign without
changing which path wins any tie that did not overflow, so no query plan
changes.
Comparing the joined strings with `String#compareTo` would look more
thorough, since it also removes hash collisions, but it changes the sort key
and therefore the winner of every equal-cost tie. One such tie is real:
ArrowNative to VanillaRow has two paths costing 15 each, `LoadArrowData` plus
`ColumnarToRow` versus `ArrowColumnarToVeloxColumnar` plus
`VeloxColumnarToRow`, and their hash codes are close enough that the
subtraction was already correct. Switching to `compareTo` flips it and rewrites
the plan for every query where an Arrow-native operator feeds a row operator.
Two limits are worth recording rather than fixing here, since removing
either changes tie winners: distinct name sequences can share a hash code, and
`mkString` joins without a separator, so `Seq("Load", "ArrowData")` and
`Seq("LoadArrow", "Data")` collapse to one string. A tie then falls back on
`mutable.Map` iteration order, which is what the existing "To make the output
order stable" comment overpromises.
--
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]
