You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by xinyunh <gi...@git.apache.org> on 2014/08/22 20:20:34 UTC
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
GitHub user xinyunh opened a pull request:
https://github.com/apache/spark/pull/2099
[SPARK-3176] Implement 'POWER', 'ABS and 'LAST' for sql
Add support for the mathematical function "POWER" and "ABS" and the analytic function "last" to return a subset of the rows satisfying a query within spark sql. Test-cases included.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/Huawei-Spark/spark sqlTest
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/2099.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #2099
----
commit c37ec6e2a9c8f5e522ed37cc3bfa41be696433a5
Author: xinyunh <xi...@huawei.com>
Date: 2014-08-14T20:13:02Z
add 'Last' component
commit 9becb24fdc42800e9c2211c4090bf83bb23efbbe
Author: xinyunh <xi...@huawei.com>
Date: 2014-08-14T23:10:28Z
fix the bug in 'Last' component
commit c68f3cabb4fe7447f2d7982f23e26d582358ca63
Author: bomeng <golf8lover>
Date: 2014-08-20T18:16:35Z
add abs() function support
commit fe22feed44737f397cbacba737dd6ed0ec61ef4c
Author: bomeng <golf8lover>
Date: 2014-08-20T18:35:48Z
add abs() function support
commit f7d501c5033bacdfabbf0898cc1bd7fecf4f3c8f
Author: Bo Meng <bo...@huawei.com>
Date: 2014-08-20T21:16:46Z
add math 'power' function
commit 024b5bbf2af1b4057fb9d182da4d11357411b7e3
Author: Bo Meng <bo...@huawei.com>
Date: 2014-08-21T17:51:40Z
handle the non-numeric string
commit 7570fa64b526f92240e6a8052cc74d00b97c6d09
Author: xinyunh <xi...@gmail.com>
Date: 2014-08-22T18:16:40Z
Upload testcases and optimize the code
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16923732
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numericOperations.scala ---
@@ -0,0 +1,78 @@
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.sql.catalyst.types.{DoubleType, StringType, DataType}
+import org.apache.spark.sql.catalyst.analysis.UnresolvedException
+import scala.math.pow
+
+trait NumberConversionExpression {
+ self: UnaryExpression =>
+
+ type EvaluatedType = Any
+
+ def convert(v: String): Any
+
+ override def foldable: Boolean = child.foldable
+ def nullable: Boolean = child.nullable
+ def dataType: DataType = StringType
+
+ override def eval(input: Row): Any = {
+ val evaluated = child.eval(input)
+ if (evaluated == null) {
+ null
+ } else {
+ convert(evaluated.toString)
--- End diff --
Ok, will convert it to Double directly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54202068
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/19596/consoleFull) for PR 2099 at commit [`8843643`](https://github.com/apache/spark/commit/88436434676509ee00884aae66fb087562951721).
* This patch **passes** unit tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression] `
* `case class LastFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction `
* `case class Abs(child: Expression) extends UnaryExpression `
* `case class Power(base: Expression, exponent: Expression) extends Expression `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54184669
The link to the test results is in Spark QA's latest message: "QA tests have finished"
It looks like you have a Scala style problem. You can run tests locally by running `./dev/run-tests` to check for these things before Spark QA gets to them.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16690778
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregates.scala ---
@@ -409,3 +424,15 @@ case class FirstFunction(expr: Expression, base: AggregateExpression) extends Ag
override def eval(input: Row): Any = result
}
+
+case class LastFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction {
+ def this() = this(null, null) // Required for serialization.
+
+ var result: Any = null
+
+ override def update(input: Row): Unit = {
+ result = input
+ }
+
+ override def eval(input: Row): Any = if (result != null) expr.eval(result.asInstanceOf[Row]) else null
+}
--- End diff --
End files with new lines. (Run `sbt/sbt scalastyle` to check for these types of style violations).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16923701
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregates.scala ---
@@ -409,3 +424,15 @@ case class FirstFunction(expr: Expression, base: AggregateExpression) extends Ag
override def eval(input: Row): Any = result
}
+
+case class LastFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction {
+ def this() = this(null, null) // Required for serialization.
+
+ var result: Any = null
+
+ override def update(input: Row): Unit = {
+ result = input
+ }
+
+ override def eval(input: Row): Any = if (result != null) expr.eval(result.asInstanceOf[Row]) else null
+}
--- End diff --
Ok. BTW, need I create a new pull request? Or just updating this branch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54336477
OK. Thus, I should remove the POWER part from this pull request and create another pull request which includes the POWER, correct? Need I also split the issue on Jira?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16690875
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numericOperations.scala ---
@@ -0,0 +1,78 @@
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.sql.catalyst.types.{DoubleType, StringType, DataType}
+import org.apache.spark.sql.catalyst.analysis.UnresolvedException
+import scala.math.pow
+
+trait NumberConversionExpression {
+ self: UnaryExpression =>
+
+ type EvaluatedType = Any
+
+ def convert(v: String): Any
+
+ override def foldable: Boolean = child.foldable
+ def nullable: Boolean = child.nullable
+ def dataType: DataType = StringType
+
+ override def eval(input: Row): Any = {
+ val evaluated = child.eval(input)
+ if (evaluated == null) {
+ null
+ } else {
+ convert(evaluated.toString)
--- End diff --
Why are you converting the numbers to a String? This is a pretty expensive operation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16691093
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numericOperations.scala ---
@@ -0,0 +1,78 @@
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.sql.catalyst.types.{DoubleType, StringType, DataType}
+import org.apache.spark.sql.catalyst.analysis.UnresolvedException
+import scala.math.pow
+
+trait NumberConversionExpression {
+ self: UnaryExpression =>
+
+ type EvaluatedType = Any
+
+ def convert(v: String): Any
+
+ override def foldable: Boolean = child.foldable
+ def nullable: Boolean = child.nullable
+ def dataType: DataType = StringType
+
+ override def eval(input: Row): Any = {
+ val evaluated = child.eval(input)
+ if (evaluated == null) {
+ null
+ } else {
+ convert(evaluated.toString)
+ }
+ }
+}
+/**
+ * A function that get the absolute value of the numeric value.
+ */
+case class Abs(child: Expression) extends UnaryExpression with NumberConversionExpression {
+ def parseDouble(s: String) = try { Some(s.toDouble) } catch { case _ => None }
+
+ def convert(v: String): Any = {
+ parseDouble(v) match {
+ case Some(s) => s.abs
+ case None => null
+ }
+ }
+
+ override def toString() = s"Abs($child)"
+}
+
+/**
+ * A function that get the power value of two parameters.
+ * First one is taken as base while second one taken as exponent
+ */
+case class Power(base: Expression, exponent: Expression) extends Expression {
+
+ type EvaluatedType = Any
+
+ def nullable: Boolean = base.nullable || exponent.nullable
+
+ override def children = base :: exponent :: Nil
+ def references = children.flatMap(_.references).toSet
+
+ def dataType: DataType = {
+ if (!resolved) throw new UnresolvedException(this, s"Cannot resolve since $children are not resolved")
+ DoubleType
+ }
+
+ override def eval(input: Row): Any = {
+ def convertToDouble(num: EvaluatedType): Double = {
+ num match {
+ case d:Double => d
+ case i:Integer => i.doubleValue()
+ case f:Float => f.toDouble
+ }
+ }
+
+ val base_v = base.eval(input)
+ val exponent_v = exponent.eval(input)
+
+ if ((base_v == null) || (exponent_v == null)) null
+ else pow(convertToDouble(base_v), convertToDouble(exponent_v))
+ }
+
+ override def toString = s"Power($base, $exponent)"
+}
--- End diff --
add newline.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'ABS and 'LAST' for sql
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-55059973
The test failure was unrelated. I resolved the conflict and tested locally. Merged to master. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-53361706
Thanks for working on this! I made a few comments on the implementation.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'ABS and 'LAST' for sql
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54915989
Mind fixing the conflict so we can merge this? Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-53942469
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'ABS and 'LAST' for sql
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54663384
Sorry, I forgot
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'ABS and 'LAST' for sql
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54924579
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20019/consoleFull) for PR 2099 at commit [`71d15e7`](https://github.com/apache/spark/commit/71d15e7eb757e32e6fa0c47425905f7cd58d9bee).
* This patch **fails** unit tests.
* This patch **does not** merge cleanly!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'ABS and 'LAST' for sql
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54921008
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/20019/consoleFull) for PR 2099 at commit [`71d15e7`](https://github.com/apache/spark/commit/71d15e7eb757e32e6fa0c47425905f7cd58d9bee).
* This patch **does not** merge cleanly!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'ABS and 'LAST' for sql
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-55043188
Why it fails while I only changed the name of title?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16923882
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numericOperations.scala ---
@@ -0,0 +1,78 @@
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.sql.catalyst.types.{DoubleType, StringType, DataType}
+import org.apache.spark.sql.catalyst.analysis.UnresolvedException
+import scala.math.pow
+
+trait NumberConversionExpression {
+ self: UnaryExpression =>
+
+ type EvaluatedType = Any
+
+ def convert(v: String): Any
+
+ override def foldable: Boolean = child.foldable
+ def nullable: Boolean = child.nullable
+ def dataType: DataType = StringType
+
+ override def eval(input: Row): Any = {
+ val evaluated = child.eval(input)
+ if (evaluated == null) {
+ null
+ } else {
+ convert(evaluated.toString)
+ }
+ }
+}
+/**
+ * A function that get the absolute value of the numeric value.
+ */
+case class Abs(child: Expression) extends UnaryExpression with NumberConversionExpression {
+ def parseDouble(s: String) = try { Some(s.toDouble) } catch { case _ => None }
+
+ def convert(v: String): Any = {
+ parseDouble(v) match {
+ case Some(s) => s.abs
+ case None => null
+ }
+ }
+
+ override def toString() = s"Abs($child)"
+}
+
+/**
+ * A function that get the power value of two parameters.
+ * First one is taken as base while second one taken as exponent
+ */
+case class Power(base: Expression, exponent: Expression) extends Expression {
+
+ type EvaluatedType = Any
+
+ def nullable: Boolean = base.nullable || exponent.nullable
+
+ override def children = base :: exponent :: Nil
+ def references = children.flatMap(_.references).toSet
+
+ def dataType: DataType = {
+ if (!resolved) throw new UnresolvedException(this, s"Cannot resolve since $children are not resolved")
+ DoubleType
--- End diff --
It would be good to investigate what other systems do in this case or if the SQL standard says something.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54337055
Yes please. We try to keep one JIRA to one PR in generall.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16690818
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
@@ -323,6 +328,10 @@ class SqlParser extends StandardTokenParsers with PackratParsers {
(SUBSTR | SUBSTRING) ~> "(" ~> expression ~ "," ~ expression ~ "," ~ expression <~ ")" ^^ {
case s ~ "," ~ p ~ "," ~ l => Substring(s,p,l)
} |
+ ABS ~> "(" ~> expression <~ ")" ^^ { case exp => Abs(exp) } |
+ (POW | POWER) ~> "(" ~> expression ~ "," ~ expression <~ ")" ^^ {
--- End diff --
Is POW and POWER standard SQL? If so we should have test cases for both.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16928036
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregates.scala ---
@@ -409,3 +424,15 @@ case class FirstFunction(expr: Expression, base: AggregateExpression) extends Ag
override def eval(input: Row): Any = result
}
+
+case class LastFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction {
+ def this() = this(null, null) // Required for serialization.
+
+ var result: Any = null
+
+ override def update(input: Row): Unit = {
+ result = input
+ }
+
+ override def eval(input: Row): Any = if (result != null) expr.eval(result.asInstanceOf[Row]) else null
+}
--- End diff --
Updating the branch will automatically include the new commits in this pull request.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54186634
Thank you! Fixed this code style issue.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-53101752
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-53942549
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/19506/consoleFull) for PR 2099 at commit [`39f0309`](https://github.com/apache/spark/commit/39f0309195eb162f8e276bf758600e70ba39ad49).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16923632
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numericOperations.scala ---
@@ -0,0 +1,78 @@
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.sql.catalyst.types.{DoubleType, StringType, DataType}
+import org.apache.spark.sql.catalyst.analysis.UnresolvedException
+import scala.math.pow
+
+trait NumberConversionExpression {
+ self: UnaryExpression =>
+
+ type EvaluatedType = Any
+
+ def convert(v: String): Any
+
+ override def foldable: Boolean = child.foldable
+ def nullable: Boolean = child.nullable
+ def dataType: DataType = StringType
+
+ override def eval(input: Row): Any = {
+ val evaluated = child.eval(input)
+ if (evaluated == null) {
+ null
+ } else {
+ convert(evaluated.toString)
+ }
+ }
+}
+/**
+ * A function that get the absolute value of the numeric value.
+ */
+case class Abs(child: Expression) extends UnaryExpression with NumberConversionExpression {
+ def parseDouble(s: String) = try { Some(s.toDouble) } catch { case _ => None }
+
+ def convert(v: String): Any = {
+ parseDouble(v) match {
+ case Some(s) => s.abs
+ case None => null
+ }
+ }
+
+ override def toString() = s"Abs($child)"
+}
+
+/**
+ * A function that get the power value of two parameters.
+ * First one is taken as base while second one taken as exponent
+ */
+case class Power(base: Expression, exponent: Expression) extends Expression {
+
+ type EvaluatedType = Any
+
+ def nullable: Boolean = base.nullable || exponent.nullable
+
+ override def children = base :: exponent :: Nil
+ def references = children.flatMap(_.references).toSet
+
+ def dataType: DataType = {
+ if (!resolved) throw new UnresolvedException(this, s"Cannot resolve since $children are not resolved")
+ DoubleType
--- End diff --
But there is a question, when I input, for example POWER(3, 0.5), the return result could not be the INT type. Should I try convert it to base's datatype as long as it could?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16691086
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numericOperations.scala ---
@@ -0,0 +1,78 @@
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.sql.catalyst.types.{DoubleType, StringType, DataType}
+import org.apache.spark.sql.catalyst.analysis.UnresolvedException
+import scala.math.pow
+
+trait NumberConversionExpression {
+ self: UnaryExpression =>
+
+ type EvaluatedType = Any
+
+ def convert(v: String): Any
+
+ override def foldable: Boolean = child.foldable
+ def nullable: Boolean = child.nullable
+ def dataType: DataType = StringType
+
+ override def eval(input: Row): Any = {
+ val evaluated = child.eval(input)
+ if (evaluated == null) {
+ null
+ } else {
+ convert(evaluated.toString)
+ }
+ }
+}
+/**
+ * A function that get the absolute value of the numeric value.
+ */
+case class Abs(child: Expression) extends UnaryExpression with NumberConversionExpression {
+ def parseDouble(s: String) = try { Some(s.toDouble) } catch { case _ => None }
+
+ def convert(v: String): Any = {
+ parseDouble(v) match {
+ case Some(s) => s.abs
+ case None => null
+ }
+ }
+
+ override def toString() = s"Abs($child)"
+}
+
+/**
+ * A function that get the power value of two parameters.
+ * First one is taken as base while second one taken as exponent
+ */
+case class Power(base: Expression, exponent: Expression) extends Expression {
+
+ type EvaluatedType = Any
+
+ def nullable: Boolean = base.nullable || exponent.nullable
+
+ override def children = base :: exponent :: Nil
+ def references = children.flatMap(_.references).toSet
+
+ def dataType: DataType = {
+ if (!resolved) throw new UnresolvedException(this, s"Cannot resolve since $children are not resolved")
+ DoubleType
--- End diff --
Should this always return a double? It seems like in other SQL dialects the return type is the same as `base`'s datatype
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16923739
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numericOperations.scala ---
@@ -0,0 +1,78 @@
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.sql.catalyst.types.{DoubleType, StringType, DataType}
+import org.apache.spark.sql.catalyst.analysis.UnresolvedException
+import scala.math.pow
+
+trait NumberConversionExpression {
+ self: UnaryExpression =>
+
+ type EvaluatedType = Any
+
+ def convert(v: String): Any
+
+ override def foldable: Boolean = child.foldable
+ def nullable: Boolean = child.nullable
+ def dataType: DataType = StringType
+
+ override def eval(input: Row): Any = {
+ val evaluated = child.eval(input)
+ if (evaluated == null) {
+ null
+ } else {
+ convert(evaluated.toString)
+ }
+ }
+}
+/**
+ * A function that get the absolute value of the numeric value.
+ */
+case class Abs(child: Expression) extends UnaryExpression with NumberConversionExpression {
+ def parseDouble(s: String) = try { Some(s.toDouble) } catch { case _ => None }
+
+ def convert(v: String): Any = {
+ parseDouble(v) match {
+ case Some(s) => s.abs
+ case None => null
+ }
+ }
+
+ override def toString() = s"Abs($child)"
+}
+
+/**
+ * A function that get the power value of two parameters.
+ * First one is taken as base while second one taken as exponent
+ */
+case class Power(base: Expression, exponent: Expression) extends Expression {
+
+ type EvaluatedType = Any
+
+ def nullable: Boolean = base.nullable || exponent.nullable
+
+ override def children = base :: exponent :: Nil
+ def references = children.flatMap(_.references).toSet
+
+ def dataType: DataType = {
+ if (!resolved) throw new UnresolvedException(this, s"Cannot resolve since $children are not resolved")
+ DoubleType
+ }
+
+ override def eval(input: Row): Any = {
+ def convertToDouble(num: EvaluatedType): Double = {
+ num match {
+ case d:Double => d
+ case i:Integer => i.doubleValue()
+ case f:Float => f.toDouble
+ }
+ }
+
+ val base_v = base.eval(input)
+ val exponent_v = exponent.eval(input)
+
+ if ((base_v == null) || (exponent_v == null)) null
+ else pow(convertToDouble(base_v), convertToDouble(exponent_v))
+ }
+
+ override def toString = s"Power($base, $exponent)"
+}
--- End diff --
Ok, will do it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54573623
Mind fixing the title to remove Power?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54184220
May I know in which test cases do we failed?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-53941443
I have made some changes according to the comments.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'ABS and 'LAST' for sql
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54917634
ok to test
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'ABS and 'LAST' for sql
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54694404
Can one of the admins verify this patch?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'ABS and 'LAST' for sql
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/2099
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54186757
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/19596/consoleFull) for PR 2099 at commit [`8843643`](https://github.com/apache/spark/commit/88436434676509ee00884aae66fb087562951721).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16690992
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numericOperations.scala ---
@@ -0,0 +1,78 @@
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.sql.catalyst.types.{DoubleType, StringType, DataType}
+import org.apache.spark.sql.catalyst.analysis.UnresolvedException
+import scala.math.pow
+
+trait NumberConversionExpression {
--- End diff --
Instead of creating another trait here, I think we should try an use the existing infrastructure based on Scala's `numeric` class. I think for example that Abs's `eval` function could just be
```scala
def eval(input: Row) = n1(child, input, _.abs(_))
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-53942584
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/19506/consoleFull) for PR 2099 at commit [`39f0309`](https://github.com/apache/spark/commit/39f0309195eb162f8e276bf758600e70ba39ad49).
* This patch **fails** unit tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression] `
* `case class LastFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction `
* `case class Abs(child: Expression) extends UnaryExpression `
* `case class Power(base: Expression, exponent: Expression) extends Expression `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54358675
[QA tests have finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/19675/consoleFull) for PR 2099 at commit [`71d15e7`](https://github.com/apache/spark/commit/71d15e7eb757e32e6fa0c47425905f7cd58d9bee).
* This patch **passes** unit tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class Last(child: Expression) extends PartialAggregate with trees.UnaryNode[Expression] `
* `case class LastFunction(expr: Expression, base: AggregateExpression) extends AggregateFunction `
* `case class Abs(child: Expression) extends UnaryExpression `
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54342991
I have split the POWER to PR #2252, Jira number is 3379
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54343539
[QA tests have started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/19675/consoleFull) for PR 2099 at commit [`71d15e7`](https://github.com/apache/spark/commit/71d15e7eb757e32e6fa0c47425905f7cd58d9bee).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by nchammas <gi...@git.apache.org>.
Github user nchammas commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16928085
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/numericOperations.scala ---
@@ -0,0 +1,78 @@
+package org.apache.spark.sql.catalyst.expressions
+
+import org.apache.spark.sql.catalyst.types.{DoubleType, StringType, DataType}
+import org.apache.spark.sql.catalyst.analysis.UnresolvedException
+import scala.math.pow
+
+trait NumberConversionExpression {
+ self: UnaryExpression =>
+
+ type EvaluatedType = Any
+
+ def convert(v: String): Any
+
+ override def foldable: Boolean = child.foldable
+ def nullable: Boolean = child.nullable
+ def dataType: DataType = StringType
+
+ override def eval(input: Row): Any = {
+ val evaluated = child.eval(input)
+ if (evaluated == null) {
+ null
+ } else {
+ convert(evaluated.toString)
+ }
+ }
+}
+/**
+ * A function that get the absolute value of the numeric value.
+ */
+case class Abs(child: Expression) extends UnaryExpression with NumberConversionExpression {
+ def parseDouble(s: String) = try { Some(s.toDouble) } catch { case _ => None }
+
+ def convert(v: String): Any = {
+ parseDouble(v) match {
+ case Some(s) => s.abs
+ case None => null
+ }
+ }
+
+ override def toString() = s"Abs($child)"
+}
+
+/**
+ * A function that get the power value of two parameters.
+ * First one is taken as base while second one taken as exponent
+ */
+case class Power(base: Expression, exponent: Expression) extends Expression {
+
+ type EvaluatedType = Any
+
+ def nullable: Boolean = base.nullable || exponent.nullable
+
+ override def children = base :: exponent :: Nil
+ def references = children.flatMap(_.references).toSet
+
+ def dataType: DataType = {
+ if (!resolved) throw new UnresolvedException(this, s"Cannot resolve since $children are not resolved")
+ DoubleType
--- End diff --
Microsoft has some good documentation for how SQL Server handles these things. As an established and very popular product, SQL Server could provide y'all with a good reference implementation for this behavior.
From their [documentation on `POWER()`](http://msdn.microsoft.com/en-us/library/ms174276.aspx):
> Returns the same type as submitted in _float_expression_. For example, if a **decimal**(2,0) is submitted as _float_expression_, the result returned is **decimal**(2,0).
There are a few good examples that follow.
Microsoft also has some good documentation on [how precision, scale, and length are calculated for results of arithmetic operations](http://msdn.microsoft.com/en-us/library/ms190476.aspx).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by xinyunh <gi...@git.apache.org>.
Github user xinyunh commented on a diff in the pull request:
https://github.com/apache/spark/pull/2099#discussion_r16923711
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/SqlParser.scala ---
@@ -323,6 +328,10 @@ class SqlParser extends StandardTokenParsers with PackratParsers {
(SUBSTR | SUBSTRING) ~> "(" ~> expression ~ "," ~ expression ~ "," ~ expression <~ ")" ^^ {
case s ~ "," ~ p ~ "," ~ l => Substring(s,p,l)
} |
+ ABS ~> "(" ~> expression <~ ")" ^^ { case exp => Abs(exp) } |
+ (POW | POWER) ~> "(" ~> expression ~ "," ~ expression <~ ")" ^^ {
--- End diff --
Ok, will do it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-3176] Implement 'POWER', 'ABS and 'LAST...
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/2099#issuecomment-54248928
The implementation of `Last` and `Abs` look great to me. I think we could merge them now. Regarding power, I'm concerned that we aren't handling all of the various cases (i.e. what about Decimal?). What do you think about splitting that off into its own PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org