Fix ClassCastException for unsupported TypedImperativeAggregate functions #3283
Conversation
…ypedImperativeAgg Signed-off-by: sperlingxx <lovedreamf@gmail.com>
|
build |
| agg.aggregateExpressions.zipWithIndex.foreach { | ||
| case (expr, i) if expr.aggregateFunction.isInstanceOf[TypedImperativeAggregate[_]] => | ||
| aggregateExpressions.map(_.childExprs.head).foreach { | ||
| case aggMeta: TypedImperativeAggExprMeta[_] => |
There was a problem hiding this comment.
Nit, this removes the need for the asInstanceOf for wrapped:
case aggMeta: TypedImperativeAggExprMeta[TypedImperativeAggregate[_]] =>
| @@ -547,6 +547,20 @@ def test_hash_groupby_collect_partial_replace_fallback_with_other_agg(conf, aqe_ | |||
| group by k1""", | |||
| conf=local_conf) | |||
|
|
|||
| @ignore_order(local=True) | |||
There was a problem hiding this comment.
I believe approx_percentile is coming, and when it does show up we will fail this test and then whoever implements the function needs to figure out how to address, it would be better if we force the aggregate function to not be on the GPU. Could you explicitly turn off an already working typed imperative aggregate?
There was a problem hiding this comment.
(i.e. spark.rapids.sql.expression.CollectList=false, or you should be able to get away with spark.rapids.sql.expression.ApproximatePercentile=false)
There was a problem hiding this comment.
Newbie question: Why do we allow_non_gpu for these aggs?
There was a problem hiding this comment.
The plugin can be placed in a mode that throws if some parts of the plan can't be replaced (spark.rapids.sql.test.enabled). The allow list (spark.rapids.sql.test.allowedNonGpu) specifies expressions that are OK even in this mode, as they are pertinent to the test.
There was a problem hiding this comment.
The spark.rapids.sql.expression.ApproximatePercentile=false is not going to work for actually testing this. We have to have a TypedImperativeAggregate that does not exist in the GpuOverrides mapping. Because even with that config it would still create a TypedImperativeAggExprMeta to wrap it. It would just be turned off.
I would propose we go with an API that is less likely we are going to support any time soon. Specifically a udaf, but that is really hard to do inside the python tests. As it is I am fine with this for now, but looking at the list of TypedImperativeAggregate functions there are not a lot to choose from.
| @@ -547,6 +547,20 @@ def test_hash_groupby_collect_partial_replace_fallback_with_other_agg(conf, aqe_ | |||
| group by k1""", | |||
| conf=local_conf) | |||
|
|
|||
| @ignore_order(local=True) | |||
There was a problem hiding this comment.
The spark.rapids.sql.expression.ApproximatePercentile=false is not going to work for actually testing this. We have to have a TypedImperativeAggregate that does not exist in the GpuOverrides mapping. Because even with that config it would still create a TypedImperativeAggExprMeta to wrap it. It would just be turned off.
I would propose we go with an API that is less likely we are going to support any time soon. Specifically a udaf, but that is really hard to do inside the python tests. As it is I am fine with this for now, but looking at the list of TypedImperativeAggregate functions there are not a lot to choose from.
Signed-off-by: sperlingxx lovedreamf@gmail.com
Fixes #3253
The ClassCastException occurred during overriding the buffer types of TypedImperativeAggregate functions. The code for overriding assumed that
TypedImperativeAggreagtemust be wrapped byTypedImperativeAggExprMeta. But there are exceptions: forTypedImperativeAggreagtewithout GPU implementation , they are wrapped byRuleNotFoundExprMeta.