HyukjinKwon commented on a change in pull request #35534:
URL: https://github.com/apache/spark/pull/35534#discussion_r809708015
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
##########
@@ -388,29 +395,13 @@ trait RuntimeReplaceable extends UnaryExpression with
Unevaluable {
* with other databases. For example, we use this to support every, any/some
aggregates by rewriting
* them with Min and Max respectively.
*/
-trait UnevaluableAggregate extends DeclarativeAggregate {
-
- override def nullable: Boolean = true
-
- override lazy val aggBufferAttributes =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "aggBufferAttributes", this)
-
- override lazy val initialValues: Seq[Expression] =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "initialValues", this)
-
- override lazy val updateExpressions: Seq[Expression] =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "updateExpressions", this)
-
- override lazy val mergeExpressions: Seq[Expression] =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "mergeExpressions", this)
-
- override lazy val evaluateExpression: Expression =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "evaluateExpression", this)
+abstract class RuntimeReplaceableAggregate extends AggregateFunction with
RuntimeReplaceable {
Review comment:
Actually I think we can make this as a trait w/ switching
`AggregateFunction` to a trait.
```diff
-abstract class AggregateFunction extends Expression {
+trait AggregateFunction extends Expression {
```
this at least compiles fine but I would prefer to do it separately. I will
follow up after this get merged.
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
##########
@@ -388,29 +395,13 @@ trait RuntimeReplaceable extends UnaryExpression with
Unevaluable {
* with other databases. For example, we use this to support every, any/some
aggregates by rewriting
* them with Min and Max respectively.
*/
-trait UnevaluableAggregate extends DeclarativeAggregate {
-
- override def nullable: Boolean = true
-
- override lazy val aggBufferAttributes =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "aggBufferAttributes", this)
-
- override lazy val initialValues: Seq[Expression] =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "initialValues", this)
-
- override lazy val updateExpressions: Seq[Expression] =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "updateExpressions", this)
-
- override lazy val mergeExpressions: Seq[Expression] =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "mergeExpressions", this)
-
- override lazy val evaluateExpression: Expression =
- throw QueryExecutionErrors.evaluateUnevaluableAggregateUnsupportedError(
- "evaluateExpression", this)
+abstract class RuntimeReplaceableAggregate extends AggregateFunction with
RuntimeReplaceable {
Review comment:
Actually I think we can make this as a trait w/ switching
`AggregateFunction` to a trait.
```diff
-abstract class AggregateFunction extends Expression {
+trait AggregateFunction extends Expression {
```
this at least compiles fine but I would prefer to do it separately. I will
follow up after this gets merged.
--
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]