Skip to content

[SQL] Miscellaneous SQL/DF expression changes. #6754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
rxin wants to merge 6 commits into from
Closed

[SQL] Miscellaneous SQL/DF expression changes. #6754

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6b21a9b
[SQL] Miscellaneous SQL/DF expression changes.
rxin Jun 11, 2015
b201cd4
Removed if.
rxin Jun 11, 2015
de827ac
Fixed style
rxin Jun 11, 2015
3c32bbc
Fixed if.
rxin Jun 11, 2015
2647067
Promote to string type.
rxin Jun 11, 2015
35fce15
Removed println.
rxin Jun 11, 2015
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
Next Next commit
[SQL] Miscellaneous SQL/DF expression changes. 
SPARK-8201 conditional function: if
SPARK-8205 conditional function: nvl
SPARK-8208 math function: ceiling
SPARK-8210 math function: degrees
SPARK-8211 math function: radians
SPARK-8219 math function: negative
SPARK-8216 math function: rename log -> ln
SPARK-8222 math function: alias power / pow
SPARK-8225 math function: alias sign / signum
SPARK-8228 conditional function: isnull
SPARK-8229 conditional function: isnotnull
SPARK-8250 string function: alias lower/lcase
SPARK-8251 string function: alias upper / ucase
  • Loading branch information
@rxin
rxin committed Jun 11, 2015
commit 6b21a9b4990b3e468a5b7eeaaec4435d52cff5cb
20 changes: 15 additions & 5 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/FunctionRegistry.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,43 +84,51 @@ object FunctionRegistry {
type FunctionBuilder = Seq[Expression] => Expression

val expressions: Map[String, FunctionBuilder] = Map(
// Non aggregate functions
// misc non-aggregate functions
expression[Abs]("abs"),
expression[CreateArray]("array"),
expression[Coalesce]("coalesce"),
expression[Explode]("explode"),
expression[If]("if"),
expression[IsNull]("isnull"),
expression[IsNotNull]("isnotnull"),
expression[Coalesce]("nvl"),
expression[Rand]("rand"),
expression[Randn]("randn"),
expression[CreateStruct]("struct"),
expression[Sqrt]("sqrt"),

// Math functions
// math functions
expression[Acos]("acos"),
expression[Asin]("asin"),
expression[Atan]("atan"),
expression[Atan2]("atan2"),
expression[Cbrt]("cbrt"),
expression[Ceil]("ceil"),
expression[Ceil]("ceiling"),
expression[Cos]("cos"),
expression[EulerNumber]("e"),
expression[Exp]("exp"),
expression[Expm1]("expm1"),
expression[Floor]("floor"),
expression[Hypot]("hypot"),
expression[Log]("log"),
expression[Log]("ln"),
expression[Log10]("log10"),
expression[Log1p]("log1p"),
expression[UnaryMinus]("negative"),
expression[Pi]("pi"),
expression[Log2]("log2"),
expression[Pow]("pow"),
expression[Pow]("power"),
expression[Rint]("rint"),
expression[Signum]("sign"),
expression[Signum]("signum"),
expression[Sin]("sin"),
expression[Sinh]("sinh"),
expression[Tan]("tan"),
expression[Tanh]("tanh"),
expression[ToDegrees]("todegrees"),
expression[ToRadians]("toradians"),
expression[ToDegrees]("degrees"),
expression[ToRadians]("radians"),

// aggregate functions
expression[Average]("avg"),
Expand All @@ -132,10 +140,12 @@ object FunctionRegistry {
expression[Sum]("sum"),

// string functions
expression[Lower]("lcase"),
expression[Lower]("lower"),
expression[StringLength]("length"),
expression[Substring]("substr"),
expression[Substring]("substring"),
expression[Upper]("ucase"),
expression[Upper]("upper")
)

Expand Down
16 changes: 16 additions & 0 deletions sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -185,12 +185,20 @@ class ColumnExpressionSuite extends QueryTest {
checkAnswer(
nullStrings.toDF.where($"s".isNull),
nullStrings.collect().toSeq.filter(r => r.getString(1) eq null))

checkAnswer(
ctx.sql("select isnull(null), isnull(1)"),
Row(true, false))
}

test("isNotNull") {
checkAnswer(
nullStrings.toDF.where($"s".isNotNull),
nullStrings.collect().toSeq.filter(r => r.getString(1) ne null))

checkAnswer(
ctx.sql("select isnotnull(null), isnotnull('a')"),
Row(false, true))
}

test("===") {
Expand Down Expand Up @@ -393,6 +401,10 @@ class ColumnExpressionSuite extends QueryTest {
testData.select(upper(lit(null))),
(1 to 100).map(n => Row(null))
)

checkAnswer(
ctx.sql("SELECT upper('aB'), ucase('cDe')"),
Row("AB", "CDE"))
}

test("lower") {
Expand All @@ -410,6 +422,10 @@ class ColumnExpressionSuite extends QueryTest {
testData.select(lower(lit(null))),
(1 to 100).map(n => Row(null))
)

checkAnswer(
ctx.sql("SELECT lower('aB'), lcase('cDe')"),
Row("ab", "cde"))
}

test("monotonicallyIncreasingId") {
Expand Down
29 changes: 14 additions & 15 deletions sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,20 @@ class DataFrameFunctionsSuite extends QueryTest {
testData2.collect().toSeq.map(r => Row(~r.getInt(0))))
}

test("length") {
test("if function") {
Copy link
Contributor

@chenghao-intel chenghao-intel Jun 11, 2015

Choose a reason for hiding this comment

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

how about put the function in the front. e.g.
function if, function length etc. Or
conditional function if
string function length
string function trim.. ?

val df = Seq((1, 2)).toDF("a", "b")
checkAnswer(
df.selectExpr("if(a = 1, 'one', 'not_one')", "if(b = 1, 'one', 'not_one')"),
Row("one", "not_one"))
}

test("nvl function") {
checkAnswer(
ctx.sql("SELECT nvl(null, 'x'), nvl('y', 'x'), nvl(null, null)"),
Row("x", "y", null))
}

test("string length function") {
checkAnswer(
nullStrings.select(strlen($"s"), strlen("s")),
nullStrings.collect().toSeq.map { r =>
Expand All @@ -127,18 +140,4 @@ class DataFrameFunctionsSuite extends QueryTest {
Row(l)
})
}

test("log2 functions test") {
val df = Seq((1, 2)).toDF("a", "b")
checkAnswer(
df.select(log2("b") + log2("a")),
Row(1))

checkAnswer(
ctx.sql("SELECT LOG2(8)"),
Row(3))
checkAnswer(
ctx.sql("SELECT LOG2(null)"),
Row(null))
}
}
51 changes: 45 additions & 6 deletions sql/core/src/test/scala/org/apache/spark/sql/MathExpressionsSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
package org.apache.spark.sql

import org.apache.spark.sql.functions._
import org.apache.spark.sql.functions.{log => logarithm}


private object MathExpressionsTestData {
Expand Down Expand Up @@ -151,20 +152,31 @@ class MathExpressionsSuite extends QueryTest {
testOneToOneMathFunction(tanh, math.tanh)
}

test("toDeg") {
test("toDegrees") {
testOneToOneMathFunction(toDegrees, math.toDegrees)
checkAnswer(
ctx.sql("SELECT degrees(0), degrees(1), degrees(1.5)"),
Seq((1, 2)).toDF().select(toDegrees(lit(0)), toDegrees(lit(1)), toDegrees(lit(1.5)))
Copy link
Contributor

@marmbrus marmbrus Jun 11, 2015

Choose a reason for hiding this comment

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

Should these tests not be using the same code path to generate the correct answer?

Copy link
Contributor Author

@rxin rxin Jun 11, 2015

Choose a reason for hiding this comment

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

This was mainly testing function registry.

The above one (testOneToOneMathFunction) tests correctness

)
}

test("toRad") {
test("toRadians") {
testOneToOneMathFunction(toRadians, math.toRadians)
checkAnswer(
ctx.sql("SELECT radians(0), radians(1), radians(1.5)"),
Seq((1, 2)).toDF().select(toRadians(lit(0)), toRadians(lit(1)), toRadians(lit(1.5)))
)
}

test("cbrt") {
testOneToOneMathFunction(cbrt, math.cbrt)
}

test("ceil") {
test("ceil and ceiling") {
testOneToOneMathFunction(ceil, math.ceil)
checkAnswer(
ctx.sql("SELECT ceiling(0), ceiling(1), ceiling(1.5)"),
Row(0.0, 1.0, 2.0))
}

test("floor") {
Expand All @@ -183,12 +195,21 @@ class MathExpressionsSuite extends QueryTest {
testOneToOneMathFunction(expm1, math.expm1)
}

test("signum") {
test("signum / sign") {
testOneToOneMathFunction[Double](signum, math.signum)

checkAnswer(
ctx.sql("SELECT sign(10), signum(-11)"),
Row(1, -1))
}

test("pow") {
test("pow / power") {
testTwoToOneMathFunction(pow, pow, math.pow)

checkAnswer(
ctx.sql("SELECT pow(1, 2), power(2, 1)"),
Seq((1, 2)).toDF().select(pow(lit(1), lit(2)), pow(lit(2), lit(1)))
)
}

test("hypot") {
Expand All @@ -199,8 +220,12 @@ class MathExpressionsSuite extends QueryTest {
testTwoToOneMathFunction(atan2, atan2, math.atan2)
}

test("log") {
test("log / ln") {
testOneToOneNonNegativeMathFunction(org.apache.spark.sql.functions.log, math.log)
checkAnswer(
ctx.sql("SELECT ln(0), ln(1), ln(1.5)"),
Seq((1, 2)).toDF().select(logarithm(lit(0)), logarithm(lit(1)), logarithm(lit(1.5)))
)
}

test("log10") {
Expand All @@ -211,4 +236,18 @@ class MathExpressionsSuite extends QueryTest {
testOneToOneNonNegativeMathFunction(log1p, math.log1p)
}

test("log2") {
val df = Seq((1, 2)).toDF("a", "b")
Copy link
Contributor

@adrian-wang adrian-wang Jun 11, 2015

Choose a reason for hiding this comment

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

I agree that we need to add tests here since they are math expressions, but I think we'd better keep tests in this suite consistent. These tests added here are in the style of DataFrameFunctionsSuite.

Copy link
Contributor

@chenghao-intel chenghao-intel Jun 11, 2015

Choose a reason for hiding this comment

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

I think this change is a great idea, as it's more straightforward when reading the code, otherwise we probably need to jump back and forth.

Copy link
Contributor

@adrian-wang adrian-wang Jun 11, 2015

Choose a reason for hiding this comment

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

Sort of...

Copy link
Contributor Author

@rxin rxin Jun 11, 2015

Choose a reason for hiding this comment

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

I think it makes sense to put all math stuff in here, all string stuff into its own suite, etc

basically if we group expressions into files; we should also group test suites in the same way.

Copy link
Contributor

@adrian-wang adrian-wang Jun 11, 2015

Choose a reason for hiding this comment

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

That makes sense.

checkAnswer(
df.select(log2("b") + log2("a")),
Row(1))

checkAnswer(ctx.sql("SELECT LOG2(8), LOG2(null)"), Row(3, null))
}

test("negative") {
checkAnswer(
ctx.sql("SELECT negative(1), negative(0), negative(-1)"),
Row(-1, 0, 1))
}
}