li-boxuan opened a new pull request, #9716:
URL: https://github.com/apache/incubator-gluten/pull/9716

   ## What changes were proposed in this pull request?
   
   In Microsoft we have a set of internal tests that check logical links in 
final plans. Recently, when I was trying to turn on RAS by default, I saw an 
assertion failure due to missing logical link in a ProjectExecTransformer. 
After some investigation I found the problem:
   
   `CollapseProjectExecTransformer` rule collapses two projects,
   
   ```
   p1
   | p2
   ```
   
   into a single ProjectExecTransformer. Spark's `transformUpWithPruning` 
method would then 
[copy](https://github.com/apache/spark/blob/b84d3e6cca95470d3c26e76861b31308ec236e14/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala#L537-L543)
 the tags from `p1` to the new collapsed project.
   
   However, if `p1` has no tags but `p2` has, then the collapsed project would 
lose those tags. This PR fixes this problem - and now the failed test passes.
   
   The failure only happened under RAS but I think it's just a coincidence.
   
   
   ## How was this patch tested?
   
   Added an assertion to `GlutenCollapseProjectExecTransformerSuite`. 
Unfortunately this added unit test doesn't reproduce the problem I mentioned as 
it would have passed even without the fix, but I think it's at least a 
safeguard to prove there's no regression.
   
   Unfortunately I don't know how I can construct a unittest to reproduce the 
issue.
   
   


-- 
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]

Reply via email to