Skip to content
Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
29fdaba
expressionWithAlias for First, Last, StddevSamp, VarianceSamp
amanomer Dec 9, 2019
7d5f4be
Fixed errors
amanomer Dec 9, 2019
7ba7802
+constructor First
amanomer Dec 10, 2019
759262d
+constructor Last
amanomer Dec 10, 2019
5ec101f
Fixed build error
amanomer Dec 10, 2019
f210bb9
Fixed TC
amanomer Dec 10, 2019
cc234b9
ScalaStyle Fix
amanomer Dec 10, 2019
7617ec3
Fixed TC
amanomer Dec 11, 2019
92e381b
Reduce constructors
amanomer Dec 11, 2019
a2d75de
nit
amanomer Dec 11, 2019
3ef62af
Removed unnecessary changes
amanomer Dec 11, 2019
87c6ea3
Fixed TC
amanomer Dec 11, 2019
9480532
Override flatArguments in VarianceSamp, StddevSamp
amanomer Dec 11, 2019
5c540fc
override flatArguments in First, Last
amanomer Dec 12, 2019
d780dfc
override flatArguments in BoolAnd, BoolOr
amanomer Dec 12, 2019
85d9597
add assert()
amanomer Dec 12, 2019
a71e8a7
expressionWithAlias for Average, ApproximatePercentile & override nod…
amanomer Dec 13, 2019
dd2d85d
Fixes for latest update
amanomer Dec 13, 2019
c1b3afb
Fix ApproximatePercentile TC
amanomer Dec 14, 2019
e7a4e90
UT fix
amanomer Dec 14, 2019
ca886f0
nit
amanomer Dec 18, 2019
125cfac
expressionWithTreeNodeTag for ApproximatePercentile
amanomer Dec 18, 2019
4ca20f4
expressionWithTreeNodeTag for BoolAnd, BoolOr, StddevSamp and Varianc…
amanomer Dec 18, 2019
aecdd8a
expressionWithTreeNodeTag for First, Last and Average
amanomer Dec 18, 2019
bbd4397
Renaming to expressionWithTNT
amanomer Dec 18, 2019
9146913
nit
amanomer Dec 18, 2019
8e9e42b
Avoid duplicate code
amanomer Dec 18, 2019
36418e2
small fix
amanomer Dec 18, 2019
ce8ea17
move FUNC_ALIAS to FunctionRegistry
amanomer Dec 19, 2019
4b536dd
Remove expressionWithAlias
amanomer Dec 19, 2019
737f33a
revert reorder
amanomer Dec 19, 2019
1920940
override prettyName instead of nodeName
amanomer Dec 19, 2019
700a84d
Merge branch 'master' into fncAlias
amanomer Dec 19, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -692,10 +692,10 @@ class Analyzer(
// Assumption is the aggregate function ignores nulls. This is true for all current
// AggregateFunction's with the exception of First and Last in their default mode
// (which we handle) and possibly some Hive UDAF's.
case First(expr, _) =>
First(ifExpr(expr), Literal(true))
case Last(expr, _) =>
Last(ifExpr(expr), Literal(true))
case First(funcName, expr, _) =>
First(funcName, ifExpr(expr), Literal(true))
case Last(funcName, expr, _) =>
Last(funcName, ifExpr(expr), Literal(true))
case a: AggregateFunction =>
a.withNewChildren(a.children.map(ifExpr))
}.transform {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -288,11 +288,11 @@ object FunctionRegistry {
expression[CountIf]("count_if"),
expression[CovPopulation]("covar_pop"),
expression[CovSample]("covar_samp"),
expression[First]("first"),
expression[First]("first_value"),
expressionWithAlias[First]("first"),
expressionWithAlias[First]("first_value"),
expression[Kurtosis]("kurtosis"),
expression[Last]("last"),
expression[Last]("last_value"),
expressionWithAlias[Last]("last"),
expressionWithAlias[Last]("last_value"),
expression[Max]("max"),
expression[MaxBy]("max_by"),
expression[Average]("mean"),
Expand All @@ -302,14 +302,14 @@ object FunctionRegistry {
expression[Skewness]("skewness"),
expression[ApproximatePercentile]("percentile_approx"),
expression[ApproximatePercentile]("approx_percentile"),
expression[StddevSamp]("std"),
expression[StddevSamp]("stddev"),
expressionWithAlias[StddevSamp]("std"),
expressionWithAlias[StddevSamp]("stddev"),
expression[StddevPop]("stddev_pop"),
expression[StddevSamp]("stddev_samp"),
expressionWithAlias[StddevSamp]("stddev_samp"),
Comment thread
cloud-fan marked this conversation as resolved.
Outdated
expression[Sum]("sum"),
expression[VarianceSamp]("variance"),
expressionWithAlias[VarianceSamp]("variance"),
expression[VariancePop]("var_pop"),
expression[VarianceSamp]("var_samp"),
expressionWithAlias[VarianceSamp]("var_samp"),
expression[CollectList]("collect_list"),
expression[CollectSet]("collect_set"),
expression[CountMinSketchAgg]("count_min_sketch"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -431,11 +431,11 @@ object TypeCoercion {
case Sum(e @ StringType()) => Sum(Cast(e, DoubleType))
case Average(e @ StringType()) => Average(Cast(e, DoubleType))
case StddevPop(e @ StringType()) => StddevPop(Cast(e, DoubleType))
case StddevSamp(e @ StringType()) => StddevSamp(Cast(e, DoubleType))
case StddevSamp(funcName, e @ StringType()) => StddevSamp(funcName, Cast(e, DoubleType))
case UnaryMinus(e @ StringType()) => UnaryMinus(Cast(e, DoubleType))
case UnaryPositive(e @ StringType()) => UnaryPositive(Cast(e, DoubleType))
case VariancePop(e @ StringType()) => VariancePop(Cast(e, DoubleType))
case VarianceSamp(e @ StringType()) => VarianceSamp(Cast(e, DoubleType))
case VarianceSamp(funcName, e @ StringType()) => VarianceSamp(funcName, Cast(e, DoubleType))
case Skewness(e @ StringType()) => Skewness(Cast(e, DoubleType))
case Kurtosis(e @ StringType()) => Kurtosis(Cast(e, DoubleType))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ case class StddevPop(child: Expression) extends CentralMomentAgg(child) {
""",
since = "1.6.0")
// scalastyle:on line.size.limit
case class StddevSamp(child: Expression) extends CentralMomentAgg(child) {
case class StddevSamp(funcName: String, child: Expression) extends CentralMomentAgg(child) {

override protected def momentOrder = 2

Expand All @@ -174,7 +174,7 @@ case class StddevSamp(child: Expression) extends CentralMomentAgg(child) {
If(n === 1.0, Double.NaN, sqrt(m2 / (n - 1.0))))
}

override def prettyName: String = "stddev_samp"
override def nodeName: String = funcName
}

// Compute the population variance of a column
Expand Down Expand Up @@ -206,7 +206,7 @@ case class VariancePop(child: Expression) extends CentralMomentAgg(child) {
1.0
""",
since = "1.6.0")
case class VarianceSamp(child: Expression) extends CentralMomentAgg(child) {
case class VarianceSamp(funcName: String, child: Expression) extends CentralMomentAgg(child) {

override protected def momentOrder = 2

Expand All @@ -215,7 +215,7 @@ case class VarianceSamp(child: Expression) extends CentralMomentAgg(child) {
If(n === 1.0, Double.NaN, m2 / (n - 1.0)))
}

override def prettyName: String = "var_samp"
override def nodeName: String = funcName
}

@ExpressionDescription(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ import org.apache.spark.sql.types._
5
""",
since = "2.0.0")
case class First(child: Expression, ignoreNullsExpr: Expression)
case class First(funcName: String, child: Expression, ignoreNullsExpr: Expression)
extends DeclarativeAggregate with ExpectsInputTypes {

def this(child: Expression) = this(child, Literal.create(false, BooleanType))
def this(child: Expression) = this("first", child, Literal.create(false, BooleanType))

override def children: Seq[Expression] = child :: ignoreNullsExpr :: Nil

Expand Down Expand Up @@ -113,5 +113,5 @@ case class First(child: Expression, ignoreNullsExpr: Expression)

override lazy val evaluateExpression: AttributeReference = first

override def toString: String = s"first($child)${if (ignoreNulls) " ignore nulls"}"
override def toString: String = s"$funcName($child)${if (ignoreNulls) " ignore nulls"}"
}
Original file line number Diff line number Diff line change
Expand Up @@ -44,10 +44,10 @@ import org.apache.spark.sql.types._
5
""",
since = "2.0.0")
case class Last(child: Expression, ignoreNullsExpr: Expression)
case class Last(funcName: String, child: Expression, ignoreNullsExpr: Expression)
extends DeclarativeAggregate with ExpectsInputTypes {

def this(child: Expression) = this(child, Literal.create(false, BooleanType))
def this(child: Expression) = this("last", child, Literal.create(false, BooleanType))

override def children: Seq[Expression] = child :: ignoreNullsExpr :: Nil

Expand Down Expand Up @@ -111,5 +111,5 @@ case class Last(child: Expression, ignoreNullsExpr: Expression)

override lazy val evaluateExpression: AttributeReference = last

override def toString: String = s"last($child)${if (ignoreNulls) " ignore nulls"}"
override def toString: String = s"$funcName($child)${if (ignoreNulls) " ignore nulls"}"
}
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,8 @@ object RewriteDistinctAggregates extends Rule[LogicalPlan] {

// Select the result of the first aggregate in the last aggregate.
val result = AggregateExpression(
aggregate.First(evalWithinGroup(regularGroupId, operator.toAttribute), Literal(true)),
aggregate.First("first",
evalWithinGroup(regularGroupId, operator.toAttribute), Literal(true)),

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a bit verbose to always set "first" for First, so how about defining auxiliary constructor?

  def this(child: Expression, ignoreNullsExpr: Expression) =
    this("first", child, Literal.create(false, BooleanType))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

mode = Complete,
isDistinct = false)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1510,15 +1510,15 @@ class AstBuilder(conf: SQLConf) extends SqlBaseBaseVisitor[AnyRef] with Logging
*/
override def visitFirst(ctx: FirstContext): Expression = withOrigin(ctx) {
val ignoreNullsExpr = ctx.IGNORE != null
First(expression(ctx.expression), Literal(ignoreNullsExpr)).toAggregateExpression()
First("first", expression(ctx.expression), Literal(ignoreNullsExpr)).toAggregateExpression()
}

/**
* Create a [[Last]] expression.
*/
override def visitLast(ctx: LastContext): Expression = withOrigin(ctx) {
val ignoreNullsExpr = ctx.IGNORE != null
Last(expression(ctx.expression), Literal(ignoreNullsExpr)).toAggregateExpression()
Last("last", expression(ctx.expression), Literal(ignoreNullsExpr)).toAggregateExpression()
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,9 @@ import org.apache.spark.sql.types.IntegerType

class LastTestSuite extends SparkFunSuite {
val input = AttributeReference("input", IntegerType, nullable = true)()
val evaluator = DeclarativeAggregateEvaluator(Last(input, Literal(false)), Seq(input))
val evaluatorIgnoreNulls = DeclarativeAggregateEvaluator(Last(input, Literal(true)), Seq(input))
val evaluator = DeclarativeAggregateEvaluator(Last("last", input, Literal(false)), Seq(input))
val evaluatorIgnoreNulls = DeclarativeAggregateEvaluator(
Last("last", input, Literal(true)), Seq(input))

test("empty buffer") {
assert(evaluator.initialize() === InternalRow(null, false))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -787,19 +787,23 @@ class ExpressionParserSuite extends AnalysisTest {
}

test("SPARK-19526 Support ignore nulls keywords for first and last") {
assertEqual("first(a ignore nulls)", First('a, Literal(true)).toAggregateExpression())
assertEqual("first(a)", First('a, Literal(false)).toAggregateExpression())
assertEqual("last(a ignore nulls)", Last('a, Literal(true)).toAggregateExpression())
assertEqual("last(a)", Last('a, Literal(false)).toAggregateExpression())
assertEqual("first(a ignore nulls)", First("first", 'a, Literal(true)).toAggregateExpression())
assertEqual("first(a)", First("first", 'a, Literal(false)).toAggregateExpression())
assertEqual("last(a ignore nulls)", Last("last", 'a, Literal(true)).toAggregateExpression())
assertEqual("last(a)", Last("last", 'a, Literal(false)).toAggregateExpression())
}

test("Support respect nulls keywords for first_value and last_value") {
assertEqual("first_value(a ignore nulls)", First('a, Literal(true)).toAggregateExpression())
assertEqual("first_value(a respect nulls)", First('a, Literal(false)).toAggregateExpression())
assertEqual("first_value(a)", First('a, Literal(false)).toAggregateExpression())
assertEqual("last_value(a ignore nulls)", Last('a, Literal(true)).toAggregateExpression())
assertEqual("last_value(a respect nulls)", Last('a, Literal(false)).toAggregateExpression())
assertEqual("last_value(a)", Last('a, Literal(false)).toAggregateExpression())
assertEqual("first_value(a ignore nulls)",
First("first", 'a, Literal(true)).toAggregateExpression())
assertEqual("first_value(a respect nulls)",
First("first", 'a, Literal(false)).toAggregateExpression())
assertEqual("first_value(a)", First("first", 'a, Literal(false)).toAggregateExpression())
assertEqual("last_value(a ignore nulls)",
Last("last", 'a, Literal(true)).toAggregateExpression())
assertEqual("last_value(a respect nulls)",
Last("last", 'a, Literal(false)).toAggregateExpression())
assertEqual("last_value(a)", Last("last", 'a, Literal(false)).toAggregateExpression())
}

test("timestamp literals") {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,8 @@ object StatFunctions extends Logging {
stats.toLowerCase(Locale.ROOT) match {
case "count" => (child: Expression) => Count(child).toAggregateExpression()
case "mean" => (child: Expression) => Average(child).toAggregateExpression()
case "stddev" => (child: Expression) => StddevSamp(child).toAggregateExpression()
case "stddev" => (child: Expression) =>
StddevSamp("stddev_samp", child).toAggregateExpression()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shall we use stddev here?

case "min" => (child: Expression) => Min(child).toAggregateExpression()
case "max" => (child: Expression) => Max(child).toAggregateExpression()
case _ => throw new IllegalArgumentException(s"$stats is not a recognised statistic")
Expand Down
12 changes: 6 additions & 6 deletions sql/core/src/main/scala/org/apache/spark/sql/functions.scala
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,7 @@ object functions {
* @since 2.0.0
*/
def first(e: Column, ignoreNulls: Boolean): Column = withAggregateFunction {
new First(e.expr, Literal(ignoreNulls))
new First("first", e.expr, Literal(ignoreNulls))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add a constructor to provide a default name.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

/**
Expand Down Expand Up @@ -556,7 +556,7 @@ object functions {
* @since 2.0.0
*/
def last(e: Column, ignoreNulls: Boolean): Column = withAggregateFunction {
new Last(e.expr, Literal(ignoreNulls))
new Last("last", e.expr, Literal(ignoreNulls))
}

/**
Expand Down Expand Up @@ -675,7 +675,7 @@ object functions {
* @group agg_funcs
* @since 1.6.0
*/
def stddev(e: Column): Column = withAggregateFunction { StddevSamp(e.expr) }
def stddev(e: Column): Column = withAggregateFunction { StddevSamp("stddev", e.expr) }

/**
* Aggregate function: alias for `stddev_samp`.
Expand All @@ -692,7 +692,7 @@ object functions {
* @group agg_funcs
* @since 1.6.0
*/
def stddev_samp(e: Column): Column = withAggregateFunction { StddevSamp(e.expr) }
def stddev_samp(e: Column): Column = withAggregateFunction { StddevSamp("stddev_samp", e.expr) }

/**
* Aggregate function: returns the sample standard deviation of
Expand Down Expand Up @@ -759,7 +759,7 @@ object functions {
* @group agg_funcs
* @since 1.6.0
*/
def variance(e: Column): Column = withAggregateFunction { VarianceSamp(e.expr) }
def variance(e: Column): Column = withAggregateFunction { VarianceSamp("variance", e.expr) }

/**
* Aggregate function: alias for `var_samp`.
Expand All @@ -775,7 +775,7 @@ object functions {
* @group agg_funcs
* @since 1.6.0
*/
def var_samp(e: Column): Column = withAggregateFunction { VarianceSamp(e.expr) }
def var_samp(e: Column): Column = withAggregateFunction { VarianceSamp("var_samp", e.expr) }

/**
* Aggregate function: returns the unbiased variance of the values in a group.
Expand Down