wangyum commented on a change in pull request #29790:
URL: https://github.com/apache/spark/pull/29790#discussion_r492473081
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Expression.scala
##########
@@ -1048,6 +1048,11 @@ trait ComplexTypeMergingExpression extends Expression {
@transient
lazy val inputTypesForMerging: Seq[DataType] = children.map(_.dataType)
+ private lazy val internalDataType: DataType = {
Review comment:
If only change:
```scala
override def dataType: DataType = {
dataTypeCheck
inputTypesForMerging.reduceLeft(TypeCoercion.findCommonTypeDifferentOnlyInNullFlags(_,
_).get)
}
```
to
```scala
lazy val dataType: DataType = {
dataTypeCheck
inputTypesForMerging.reduceLeft(TypeCoercion.findCommonTypeDifferentOnlyInNullFlags(_,
_).get)
}
```
will throw:
```java
[error]
/Users/yumwang/opensource/spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:558:
super may not be used on lazy value dataType
[error] super.dataType.asInstanceOf[MapType]
[error] ^
[error]
/Users/yumwang/opensource/spark/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala:2077:
super may not be used on lazy value dataType
[error] super.dataType
[error] ^
[error] two errors found
```
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -368,7 +368,7 @@ case class MapEntries(child: Expression)
@transient private lazy val childDataType: MapType =
child.dataType.asInstanceOf[MapType]
- override def dataType: DataType = {
+ private lazy val internalDataType: DataType = {
Review comment:
This is to improve this case:
Benchmark code | Before this PR(Seconds) | After this PR(Seconds)
-- | -- | --
spark.range(100000000L).selectExpr("approx_count_distinct(map_entries(map(1,
id)))").collect() | 21787 | 15551
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -368,7 +368,7 @@ case class MapEntries(child: Expression)
@transient private lazy val childDataType: MapType =
child.dataType.asInstanceOf[MapType]
- override def dataType: DataType = {
+ private lazy val internalDataType: DataType = {
Review comment:
This is to improve this case:
Benchmark code | Before this PR(Milliseconds) | After this PR(Milliseconds)
-- | -- | --
spark.range(100000000L).selectExpr("approx_count_distinct(map_entries(map(1,
id)))").collect() | 21787 | 15551
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
##########
@@ -3741,11 +3746,13 @@ case class ArrayIntersect(left: Expression, right:
Expression) extends ArrayBina
case class ArrayExcept(left: Expression, right: Expression) extends
ArrayBinaryLike
with ComplexTypeMergingExpression {
- override def dataType: DataType = {
+ private lazy val internalDataType: DataType = {
dataTypeCheck
Review comment:
+1
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/subquery.scala
##########
@@ -225,10 +225,13 @@ case class ScalarSubquery(
children: Seq[Expression] = Seq.empty,
exprId: ExprId = NamedExpression.newExprId)
extends SubqueryExpression(plan, children, exprId) with Unevaluable {
- override def dataType: DataType = {
+
+ private lazy val internalDataType: DataType = {
Review comment:
I reverted this change because I did not find an expression to call this
method many times.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
