[GLUTEN-7581][CORE] Code cleanup for GlutenColumnarRule #7582
Conversation
|
Run Gluten Clickhouse CI |
| val applier = applierBuilder.apply(session) | ||
| val out = applier.apply(vanillaPlan, outputsColumnar) | ||
| out | ||
| applier.apply(vanillaPlan, outputsColumnar) |
There was a problem hiding this comment.
val out = ...
outThis code pattern is used usually because out allows developer to place a breakpoint for debugging against the output object easily. Do you happen to know an alternative way if we simplify the code?
There was a problem hiding this comment.
Yes. I understood it. We usually avoid this code pattern in Spark. Developers could evaluate the expression (e.g. out) if who want know the value.
Of course, I will respect the code pattern if Gluten has the preference.
There was a problem hiding this comment.
Let's do the simplification. Just found there is an alternative way while using Intellij IDE. Thanks.
| (child, false) | ||
| case _ => | ||
| throw new IllegalStateException( | ||
| "This should not happen. Please leave a issue at" + |
There was a problem hiding this comment.
Can you help change a issue to an issue in passing? Thanks.
| val outputsColumnar = OutputsColumnarTester.inferOutputsColumnar(plan) | ||
| val unwrapped = OutputsColumnarTester.unwrap(plan) | ||
| val vanillaPlan = Transitions.insertTransitions(unwrapped, outputsColumnar) | ||
| val (childPlan, outputsColumnar) = plan match { |
There was a problem hiding this comment.
Will renaming childPlan to originalPlan or similar help user to understand the code?
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
beliefer
changed the title
|
Run Gluten Clickhouse CI |
What changes were proposed in this pull request?
This PR proposes to make code cleanup for
GlutenColumnarRule.How was this patch tested?
integration tests