Remove GpuAttributeReference and GpuSortOrder #253
Conversation
| } else { | ||
| input: AttributeSeq): R = { | ||
| val ret = expression.transform { | ||
| case a: AttributeReference => |
There was a problem hiding this comment.
By the time we come here we will only have 2 types in the expression i.e. AttributeReference, SortOrder?
There was a problem hiding this comment.
transform walks through the tree and if the supplied pattern matches something in the tree it will be replaced with what is returned. So the goal of the code is to replace all AttributeReference instances with GpuBoundReference instances. Everything else passes through.
| @@ -118,7 +118,7 @@ abstract class GpuUnaryExpression extends UnaryExpression with GpuExpression { | |||
| def outputTypeOverride: DType = null | |||
|
|
|||
| override def columnarEval(batch: ColumnarBatch): Any = { | |||
| val input = child.asInstanceOf[GpuExpression].columnarEval(batch) | |||
| val input = child.columnarEval(batch) | |||
There was a problem hiding this comment.
Can you please clarify. From what I understand so far children could possibly contain a SortOrder??
There was a problem hiding this comment.
Nothing that is evaluated can contain a SortOrder. SortOrder is Unevaluable. So it will not work columnar or otherwise. It is up to spark to only insert it into places that it knows will work properly, like SortExec.
|
@tgravescs @jlowe Because this does have a potential impact to a lot of downstream PRs I would like to merge this in even if CI is not set up yet. All of the release changes are in except for CI. I upmerged to the latest on branch-0.2 and ran the full set of tests locally are you OK if I merge this in? |
I'm OK with it. Worst-case scenario the build is broken and we fix it with a followup or revert. We need to keep making progress. |
Signed-off-by: Jason Lowe <jlowe@nvidia.com>
This fixes #199
This fixes #187
This takes over for #244