beliefer commented on a change in pull request #35070:
URL: https://github.com/apache/spark/pull/35070#discussion_r776906643
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -209,6 +210,11 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan]
with PredicateHelper {
}
}
+ private def supportPartialAggPushDown(agg: Aggregation): Boolean = {
+ // We don't know the agg buffer of `GeneralAggregateFunc`, so can't do
partial agg push down.
+ agg.aggregateExpressions().forall(!_.isInstanceOf[GeneralAggregateFunc])
Review comment:
`agg.aggregateExpressions().exists(_.isInstanceOf[GeneralAggregateFunc])`
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/connector/expressions/expressions.scala
##########
@@ -88,9 +88,7 @@ private[sql] abstract class SingleColumnTransform(ref:
NamedReference) extends R
override def arguments: Array[Expression] = Array(ref)
- override def describe: String = name + "(" + reference.describe + ")"
-
- override def toString: String = describe
+ override def toString: String = name + "(" + reference.describe + ")"
Review comment:
It seems if describe be called, the result will different.
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala
##########
@@ -219,7 +219,11 @@ abstract class JdbcDialect extends Serializable with
Logging{
val column = quoteIdentifier(sum.column.fieldNames.head)
Some(s"SUM($distinct$column)")
case _: CountStar =>
- Some(s"COUNT(*)")
+ Some("COUNT(*)")
Review comment:
`case _: CountStar => Some("COUNT(*)")` ?
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/DataSourceStrategy.scala
##########
@@ -717,8 +717,10 @@ object DataSourceStrategy
Some(new Count(FieldReference(name), agg.isDistinct))
case _ => None
}
- case sum @ aggregate.Sum(PushableColumnWithoutNestedColumn(name), _) =>
+ case aggregate.Sum(PushableColumnWithoutNestedColumn(name), _) =>
Some(new Sum(FieldReference(name), agg.isDistinct))
+ case aggregate.Average(PushableColumnWithoutNestedColumn(name), _) =>
Review comment:
Do we need add ` if completePushdown` here?
##########
File path:
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/V2ScanRelationPushDown.scala
##########
@@ -282,7 +288,7 @@ object V2ScanRelationPushDown extends Rule[LogicalPlan]
with PredicateHelper {
}
operation
case s @ Sort(order, _, operation @ ScanOperation(_, filter, sHolder:
ScanBuilderHolder))
- if filter.isEmpty =>
+ if filter.isEmpty =>
Review comment:
I feel there should be two indent.
--
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]
