[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user asfgit closed the pull request at: https://github.com/apache/spark/pull/6725 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-113051708 Thanks - I'm going to merge this and fix the since version. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-113052722 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user davies commented on a diff in the pull request: https://github.com/apache/spark/pull/6725#discussion_r32653337 --- Diff: python/pyspark/sql/functions.py --- @@ -404,6 +405,21 @@ def when(condition, value): @since(1.4) --- End diff -- 1.5 --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111813086 [Test build #34881 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34881/console) for PR 6725 at commit [`fd01863`](https://github.com/apache/spark/commit/fd01863c7c47223c138ed267d24198511f22b3f3). * This patch **fails PySpark unit tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class Logarithm(left: Expression, right: 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111805838 [Test build #34881 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34881/consoleFull) for PR 6725 at commit [`fd01863`](https://github.com/apache/spark/commit/fd01863c7c47223c138ed267d24198511f22b3f3). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111813092 Merged build finished. Test 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111805774 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111805782 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111828049 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111828154 [Test build #34885 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34885/consoleFull) for PR 6725 at commit [`102070d`](https://github.com/apache/spark/commit/102070da2e57488911e61384c21a9e46216ea7db). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111828052 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111790522 Merged build finished. Test 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111790437 **[Test build #34867 timed out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34867/console)** for PR 6725 at commit [`6089d11`](https://github.com/apache/spark/commit/6089d11dca3a0632218293cce8fc8f2ea293b22b) after a configured wait of `175m`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111860304 [Test build #34888 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34888/console) for PR 6725 at commit [`bf96bd9`](https://github.com/apache/spark/commit/bf96bd9ece95a41fb56151d2167b957fa357136e). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class Logarithm(left: Expression, right: 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111860315 Merged build finished. Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111841603 [Test build #34885 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34885/console) for PR 6725 at commit [`102070d`](https://github.com/apache/spark/commit/102070da2e57488911e61384c21a9e46216ea7db). * This patch **fails PySpark unit tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class Logarithm(left: Expression, right: 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111843152 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111843159 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111843274 [Test build #34888 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34888/consoleFull) for PR 6725 at commit [`bf96bd9`](https://github.com/apache/spark/commit/bf96bd9ece95a41fb56151d2167b957fa357136e). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111841615 Merged build finished. Test 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111768867 log(a) should be log(e, a), where e is the natural logatirhm. so we should just have a ctor that takes one argument, and creates Log(EulerNumber, argument). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375528
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala
---
@@ -204,4 +204,22 @@ class MathFunctionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
testBinary(Atan2, math.atan2)
}
+ test(binary log) {
+val f = (c1: Double, c2: Double) = math.log(c2) / math.log(c1)
+val domain = (1 to 20).map(v = (v * 0.1, v * 0.2))
+
+domain.foreach { case (v1, v2) =
+ checkEvaluation(Logarithm(Literal(v1), Literal(v2)), f(v1 + 0.0, v2
+ 0.0), EmptyRow)
+ checkEvaluation(Logarithm(Literal(v2), Literal(v1)), f(v2 + 0.0, v1
+ 0.0), EmptyRow)
+}
+// When base is null, Logarithm is 10-based log.
--- End diff --
Maybe I misunderstand @rxin's suggestion. I think you were suggesting the
ctor that I just committed, not a default n base logarithm for log(null, a),
right?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375525
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -255,3 +265,56 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends AbstractBinaryMathExpression[Double, Double, Double](LOG) {
+ def this(child: Expression) = {
+this(Literal(math.E), child)
--- End diff --
don't use a literla, just use EulerNumber 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111771377 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request: https://github.com/apache/spark/pull/6725#discussion_r32375627 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala --- @@ -90,18 +90,26 @@ abstract class UnaryMathExpression(f: Double = Double, name: String) } /** + * A binary expression specifically for math functions that take two input parameters and + * returns one output value. + * @param name The short name of the function + */ +abstract class AbstractBinaryMathExpression[T, U, V](name: String) --- End diff -- @rxin, do you think I need to revert this? It is now just used by `Atan2`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111771375 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111776334 [Test build #34860 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34860/console) for PR 6725 at commit [`0634ef7`](https://github.com/apache/spark/commit/0634ef791184a6ade38ed91bae5c91731f3ac718). * This patch **fails Spark unit tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `class JoinedRow extends InternalRow ` * `class JoinedRow2 extends InternalRow ` * `class JoinedRow3 extends InternalRow ` * `class JoinedRow4 extends InternalRow ` * `class JoinedRow5 extends InternalRow ` * `class JoinedRow6 extends InternalRow ` * `class BaseOrdering extends Ordering[InternalRow] ` * `abstract class AbstractBinaryMathExpression[T, U, V](name: String)` * `case class Logarithm(left: Expression, right: 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111776341 Merged build finished. Test 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375494
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
--- End diff --
ok. it solves this problem.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375557
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala
---
@@ -204,4 +204,22 @@ class MathFunctionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
testBinary(Atan2, math.atan2)
}
+ test(binary log) {
+val f = (c1: Double, c2: Double) = math.log(c2) / math.log(c1)
+val domain = (1 to 20).map(v = (v * 0.1, v * 0.2))
+
+domain.foreach { case (v1, v2) =
+ checkEvaluation(Logarithm(Literal(v1), Literal(v2)), f(v1 + 0.0, v2
+ 0.0), EmptyRow)
+ checkEvaluation(Logarithm(Literal(v2), Literal(v1)), f(v2 + 0.0, v1
+ 0.0), EmptyRow)
+}
+// When base is null, Logarithm is 10-based log.
--- End diff --
if the 1st argument is null, we definitely shouldn't use 10 base
implicitly. can you take a look at what hive returns in that case? maybe just
null.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111778145 [Test build #34867 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34867/consoleFull) for PR 6725 at commit [`6089d11`](https://github.com/apache/spark/commit/6089d11dca3a0632218293cce8fc8f2ea293b22b). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375281
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends BinaryMathExpression((c1, c2) = math.log(c2) / math.log(c1),
LOG) {
--- End diff --
I would argue that that function is also implemented in a confusing way.
We should not shoehorn things into the class hierarchy if its going to result
hard to follow code. I'd rather we have small amounts of code duplication.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375290
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala
---
@@ -204,4 +204,22 @@ class MathFunctionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
testBinary(Atan2, math.atan2)
}
+ test(binary log) {
+val f = (c1: Double, c2: Double) = math.log(c2) / math.log(c1)
+val domain = (1 to 20).map(v = (v * 0.1, v * 0.2))
+
+domain.foreach { case (v1, v2) =
+ checkEvaluation(Logarithm(Literal(v1), Literal(v2)), f(v1 + 0.0, v2
+ 0.0), EmptyRow)
+ checkEvaluation(Logarithm(Literal(v2), Literal(v1)), f(v2 + 0.0, v1
+ 0.0), EmptyRow)
+}
+// When base is null, Logarithm is 10-based log.
--- End diff --
If so, we should not support the default 10 based logarithm? Since it is
suggested by @rxin, how do you think?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375357
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends BinaryMathExpression((c1, c2) = math.log(c2) / math.log(c1),
LOG) {
--- End diff --
OK. Sounds reasonable. I think I can refactor this part of codes a little.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375476
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
--- End diff --
take a look at https://github.com/apache/spark/pull/6806
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375580
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala
---
@@ -204,4 +204,22 @@ class MathFunctionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
testBinary(Atan2, math.atan2)
}
+ test(binary log) {
+val f = (c1: Double, c2: Double) = math.log(c2) / math.log(c1)
+val domain = (1 to 20).map(v = (v * 0.1, v * 0.2))
+
+domain.foreach { case (v1, v2) =
+ checkEvaluation(Logarithm(Literal(v1), Literal(v2)), f(v1 + 0.0, v2
+ 0.0), EmptyRow)
+ checkEvaluation(Logarithm(Literal(v2), Literal(v1)), f(v2 + 0.0, v1
+ 0.0), EmptyRow)
+}
+// When base is null, Logarithm is 10-based log.
--- End diff --
Yes. it is null. I will update this.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request: https://github.com/apache/spark/pull/6725#discussion_r32376356 --- Diff: python/pyspark/sql/functions.py --- @@ -143,7 +143,8 @@ def _(): 'atan2': 'Returns the angle theta from the conversion of rectangular coordinates (x, y) to' + 'polar coordinates (r, theta).', 'hypot': 'Computes `sqrt(a^2^ + b^2^)` without intermediate overflow or underflow.', -'pow': 'Returns the value of the first argument raised to the power of the second argument.' +'pow': 'Returns the value of the first argument raised to the power of the second argument.', +'log': 'Returns the first argument-based logarithm of the second argument', --- End diff -- can we define this function directly instead of using this magic? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111769155 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111769142 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111777873 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111777875 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111780568 Merged build finished. Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111769616 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111769620 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375539
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -255,3 +265,56 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends AbstractBinaryMathExpression[Double, Double, Double](LOG) {
+ def this(child: Expression) = {
+this(Literal(math.E), child)
--- End diff --
ok. haven't noticed there is EulerNumber.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request: https://github.com/apache/spark/pull/6725#discussion_r32375632 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala --- @@ -90,18 +90,26 @@ abstract class UnaryMathExpression(f: Double = Double, name: String) } /** + * A binary expression specifically for math functions that take two input parameters and + * returns one output value. + * @param name The short name of the function + */ +abstract class AbstractBinaryMathExpression[T, U, V](name: String) --- End diff -- yes let's not complicate the type hierarchy just for one thing --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111771425 [Test build #34863 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34863/consoleFull) for PR 6725 at commit [`bc89597`](https://github.com/apache/spark/commit/bc89597d21069fda9ac0801831290da49f7a0edf). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375269
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends BinaryMathExpression((c1, c2) = math.log(c2) / math.log(c1),
LOG) {
--- End diff --
It should be ok because there is other binary math expression `Atan2` that
overrides `eval` because its eval behavior is different than the default one in
`BinaryMathExpression`.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111769207 [Test build #34860 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34860/consoleFull) for PR 6725 at commit [`0634ef7`](https://github.com/apache/spark/commit/0634ef791184a6ade38ed91bae5c91731f3ac718). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111769216 @rxin it is. Currently log(a) will create Log(a), which is natural logarithm. The issue now is how to handle `log(null, a)`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111769677 [Test build #34862 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34862/consoleFull) for PR 6725 at commit [`db7dc38`](https://github.com/apache/spark/commit/db7dc38084950598865e2ce18f01c59929f2d1d7). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32375635
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1084,6 +1084,22 @@ object functions {
def log(columnName: String): Column = log(Column(columnName))
/**
+ * Returns the first argument-base logarithm of the second argument.
+ *
+ * @group math_funcs
+ * @since 1.4.0
+ */
+ def log(base: Double, a: Column): Column = Logarithm(lit(base).expr,
a.expr)
+
+ /**
+ * Returns the first argument-base logarithm of the second argument.
+ *
+ * @group math_funcs
+ * @since 1.4.0
+ */
+ def log(base: Double, a: String): Column = log(base, Column(a))
--- End diff --
can we rename a to columnName here
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111777972 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111777968 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111783117 Merged build finished. Test 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111783111 **[Test build #34862 timed out](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34862/console)** for PR 6725 at commit [`db7dc38`](https://github.com/apache/spark/commit/db7dc38084950598865e2ce18f01c59929f2d1d7) after a configured wait of `175m`. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111780478 [Test build #34863 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34863/console) for PR 6725 at commit [`bc89597`](https://github.com/apache/spark/commit/bc89597d21069fda9ac0801831290da49f7a0edf). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `class JoinedRow extends InternalRow ` * `class JoinedRow2 extends InternalRow ` * `class JoinedRow3 extends InternalRow ` * `class JoinedRow4 extends InternalRow ` * `class JoinedRow5 extends InternalRow ` * `class JoinedRow6 extends InternalRow ` * `class BaseOrdering extends Ordering[InternalRow] ` * `abstract class AbstractBinaryMathExpression[T, U, V](name: String)` * `case class Logarithm(left: Expression, right: 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32376385
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -255,3 +255,18 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends BinaryMathExpression((c1, c2) = math.log(c2) / math.log(c1),
LOG) {
+ def this(child: Expression) = {
+this(EulerNumber(), child)
+ }
+
+ override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode):
String = {
+defineCodeGen(ctx, ev, (c1, c2) = sjava.lang.Math.log($c2) /
java.lang.Math.log($c1)) + s
--- End diff --
if left == EulerNumber, can we generate the code to just use
java.lang.Math.log?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32371060
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala
---
@@ -204,4 +204,22 @@ class MathFunctionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
testBinary(Atan2, math.atan2)
}
+ test(binary log) {
+val f = (c1: Double, c2: Double) = math.log(c2) / math.log(c1)
+val domain = (1 to 20).map(v = (v * 0.1, v * 0.2))
+
+domain.foreach { case (v1, v2) =
+ checkEvaluation(Logarithm(Literal(v1), Literal(v2)), f(v1 + 0.0, v2
+ 0.0), EmptyRow)
+ checkEvaluation(Logarithm(Literal(v2), Literal(v1)), f(v2 + 0.0, v1
+ 0.0), EmptyRow)
+}
+// When base is null, Logarithm is 10-based log.
--- End diff --
We support the default base as 10.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32371101
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
--- End diff --
Because we want to support the usage of `df.selectExpr(log(a), log(2.0,
a), log(b))`.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111717437 @rxin is this ready to go? --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32371084
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1084,6 +1084,22 @@ object functions {
def log(columnName: String): Column = log(Column(columnName))
/**
+ * Returns the first argument-base logarithm of the second argument.
+ *
+ * @group math_funcs
+ * @since 1.4.0
+ */
+ def log(base: Double, a: Column): Column = Logarithm(lit(base).expr,
a.expr)
+
+ /**
+ * Returns the first argument-base logarithm of the second argument.
+ *
+ * @group math_funcs
+ * @since 1.4.0
+ */
+ def log(base: Double, a: String): Column = log(base, Column(a))
+
--- End diff --
It is suggested by @rxin. I think it is reasonable because it is hard to
have a use case to returns the logarithm of one column with another column as
base. Usually you want to compute the logarithm values for a column with the
same base.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32371138
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends BinaryMathExpression((c1, c2) = math.log(c2) / math.log(c1),
LOG) {
--- End diff --
I think `BinaryArithmetic` is more limited in its semantic meaning. As I
can tell, it is more suitable for the binary expression between two expressions
of same type. `BinaryMathExpression` is more like to represent math function
with two expressions.
Due to support the case when the left is null in that 10 base logarithm is
applied, to override `eval()` is needed here.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32373967
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala
---
@@ -204,4 +204,22 @@ class MathFunctionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
testBinary(Atan2, math.atan2)
}
+ test(binary log) {
+val f = (c1: Double, c2: Double) = math.log(c2) / math.log(c1)
+val domain = (1 to 20).map(v = (v * 0.1, v * 0.2))
+
+domain.foreach { case (v1, v2) =
+ checkEvaluation(Logarithm(Literal(v1), Literal(v2)), f(v1 + 0.0, v2
+ 0.0), EmptyRow)
+ checkEvaluation(Logarithm(Literal(v2), Literal(v1)), f(v2 + 0.0, v1
+ 0.0), EmptyRow)
+}
+// When base is null, Logarithm is 10-based log.
--- End diff --
Using null as a sentinel value to signify use the default does not match
any of the other code in Spark SQL. Typically all operations that involve null
return null.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32373942
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends BinaryMathExpression((c1, c2) = math.log(c2) / math.log(c1),
LOG) {
--- End diff --
Okay, but this is confusing. Is it this lambda that is being used for
evaluation or the `eval` function below? We should not have both if only one
is used.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32373979
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,54 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
--- End diff --
Inheriting from `BinaryMathExpression` seems like a bad idea to me. It is
forcing the arguments to have weird names and is resulting in dead 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111378648 @rxin @marmbrus I think your comments are addressed now. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32315992
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala
---
@@ -204,4 +204,22 @@ class MathFunctionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
testBinary(Atan2, math.atan2)
}
+ test(binary log) {
+val f = (c1: Double, c2: Double) = math.log(c2) / math.log(c1)
+val domain = (1 to 20).map(v = (v * 0.1, v * 0.2))
+
+domain.foreach { case (v1, v2) =
+ checkEvaluation(Logarithm(Literal(v1), Literal(v2)), f(v1 + 0.0, v2
+ 0.0), EmptyRow)
+ checkEvaluation(Logarithm(Literal(v2), Literal(v1)), f(v2 + 0.0, v1
+ 0.0), EmptyRow)
+}
+// When base is null, Logarithm is 10-based log.
--- End diff --
Why not `null` value if the base is `null`?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user viirya commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32320603
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/MathExpressionsSuite.scala ---
@@ -236,6 +236,19 @@ class MathExpressionsSuite extends QueryTest {
testOneToOneNonNegativeMathFunction(log1p, math.log1p)
}
+ test(binary log) {
+val df = Seq[(Integer, Integer)]((123, null)).toDF(a, b)
+checkAnswer(
+ df.select(org.apache.spark.sql.functions.log(a),
--- End diff --
It conflicts with Logger.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32314542
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends BinaryMathExpression((c1, c2) = math.log(c2) / math.log(c1),
LOG) {
--- End diff --
If we need to override the `eval()`, probably it's better to inherit from
`BinaryArithmetic`, not `BinaryMathExpression`, then we needn't to pass the
additional function object for its parent class.
Or can we remove the `eval()` from this class?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32315475
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1084,6 +1084,22 @@ object functions {
def log(columnName: String): Column = log(Column(columnName))
/**
+ * Returns the first argument-base logarithm of the second argument.
+ *
+ * @group math_funcs
+ * @since 1.4.0
+ */
+ def log(base: Double, a: Column): Column = Logarithm(lit(base).expr,
a.expr)
+
+ /**
+ * Returns the first argument-base logarithm of the second argument.
+ *
+ * @group math_funcs
+ * @since 1.4.0
+ */
+ def log(base: Double, a: String): Column = log(base, Column(a))
+
--- End diff --
Why don't we support specify the `base` from a column?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32315786
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,52 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
--- End diff --
Do we really need this? We should assume people will use the `ln` instead?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32315686
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/MathExpressionsSuite.scala ---
@@ -236,6 +236,19 @@ class MathExpressionsSuite extends QueryTest {
testOneToOneNonNegativeMathFunction(log1p, math.log1p)
}
+ test(binary log) {
+val df = Seq[(Integer, Integer)]((123, null)).toDF(a, b)
+checkAnswer(
+ df.select(org.apache.spark.sql.functions.log(a),
--- End diff --
Can we remove the `org.apache.spark.sql.functions.` ?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111046086 [Test build #34681 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34681/consoleFull) for PR 6725 at commit [`605574d`](https://github.com/apache/spark/commit/605574d7f234fb428d9b107c34cbefab77e2e8c5). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111045661 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111045624 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111088074 [Test build #34681 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34681/console) for PR 6725 at commit [`605574d`](https://github.com/apache/spark/commit/605574d7f234fb428d9b107c34cbefab77e2e8c5). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class Log2(child: Expression)` * `case class Logarithm(left: Expression, right: 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111088089 Merged build finished. Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111287892 Merged build finished. Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111287878 [Test build #34712 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34712/console) for PR 6725 at commit [`23c54a3`](https://github.com/apache/spark/commit/23c54a381f8a3e062982cc44bf6384c51ec5fd93). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class Logarithm(left: Expression, right: 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32275450
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,54 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
--- End diff --
Yeah, that is what I meant saying it wasn't worth whatever code reuse we
are getting. The other option would be to name the arguments and have `def
left = base`, etc.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32274856
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,54 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
--- End diff --
We seem to be doing this throughout the file, but it seems pretty confusing
to me to be using `left` and `right` instead of something like `value` and
`base`. I'm not sure this is worth whatever code reuse we are getting.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32275335
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala
---
@@ -204,4 +204,18 @@ class MathFunctionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
testBinary(Atan2, math.atan2)
}
+ test(binary log) {
+val f = (c1: Double, c2: Double) = math.log(c2) / math.log(c1)
+val domain = (1 to 20).map(v = (v * 0.1, v * 0.2))
+
+domain.foreach { case (v1, v2) =
+checkEvaluation(Logarithm(Literal(v1), Literal(v2)), f(v1 + 0.0,
v2 + 0.0), EmptyRow)
--- End diff --
Indent is wrong.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32275171
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,54 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
--- End diff --
Oh - one thing is that left/right is coming from BinaryExpression
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32275146
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,54 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
--- End diff --
+1
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32275321
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala ---
@@ -110,6 +110,19 @@ class DataFrameFunctionsSuite extends QueryTest {
testData2.collect().toSeq.map(r = Row(~r.getInt(0
}
+ test(log) {
+val df = Seq[(Integer, Integer)]((123, null)).toDF(a, b)
+checkAnswer(
+ df.select(org.apache.spark.sql.functions.log(a),
--- End diff --
Nit: When wrapping, wrap all arguments. Also the indent below is wrong.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32275207
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,54 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends BinaryMathExpression((c1, c2) = math.log(c2) / math.log(c1),
LOG) {
+ override def eval(input: Row): Any = {
+val evalE2 = right.eval(input)
+if (evalE2 == null) {
+ null
+} else {
+ val evalE1 = left.eval(input)
+ var result: Double = 0.0
+ if (evalE1 == null) {
+result = math.log(evalE2.asInstanceOf[Double])
+ } else {
+result = math.log(evalE2.asInstanceOf[Double]) /
math.log(evalE1.asInstanceOf[Double])
+ }
+ if (result.isNaN) null else result
+}
+ }
+
+ override def genCode(ctx: CodeGenContext, ev: GeneratedExpressionCode):
String = {
+if (left.dataType != right.dataType) {
+ // log.warn(s${left.dataType} != ${right.dataType})
--- End diff --
Remove
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32275133
--- Diff:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/math.scala
---
@@ -254,3 +254,54 @@ case class Pow(left: Expression, right: Expression)
}
}
+
+object Logarithm {
+ def apply(child: Expression): Expression = new Log(child)
+}
+
+case class Logarithm(left: Expression, right: Expression)
+ extends BinaryMathExpression((c1, c2) = math.log(c2) / math.log(c1),
LOG) {
+ override def eval(input: Row): Any = {
+val evalE2 = right.eval(input)
+if (evalE2 == null) {
+ null
+} else {
+ val evalE1 = left.eval(input)
+ var result: Double = 0.0
+ if (evalE1 == null) {
+result = math.log(evalE2.asInstanceOf[Double])
--- End diff --
Is it standard to assume base 10 if the exponent is null?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32275196
--- Diff:
sql/core/src/test/scala/org/apache/spark/sql/DataFrameFunctionsSuite.scala ---
@@ -110,6 +110,19 @@ class DataFrameFunctionsSuite extends QueryTest {
testData2.collect().toSeq.map(r = Row(~r.getInt(0
}
+ test(log) {
+val df = Seq[(Integer, Integer)]((123, null)).toDF(a, b)
--- End diff --
can you move this test into the math expression suite?
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user marmbrus commented on a diff in the pull request:
https://github.com/apache/spark/pull/6725#discussion_r32275894
--- Diff:
sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/MathFunctionsSuite.scala
---
@@ -204,4 +204,18 @@ class MathFunctionsSuite extends SparkFunSuite with
ExpressionEvalHelper {
testBinary(Atan2, math.atan2)
}
+ test(binary log) {
+val f = (c1: Double, c2: Double) = math.log(c2) / math.log(c1)
+val domain = (1 to 20).map(v = (v * 0.1, v * 0.2))
+
+domain.foreach { case (v1, v2) =
+checkEvaluation(Logarithm(Literal(v1), Literal(v2)), f(v1 + 0.0,
v2 + 0.0), EmptyRow)
+checkEvaluation(Logarithm(Literal(v2), Literal(v1)), f(v2 + 0.0,
v1 + 0.0), EmptyRow)
+}
+// When base is null, Logarithm is as same as Log
+checkEvaluation(Logarithm(Literal.create(null, DoubleType),
Literal(1.0)),
--- End diff --
Same comment about wrapping.
---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111365725 [Test build #34745 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34745/console) for PR 6725 at commit [`3d75bfc`](https://github.com/apache/spark/commit/3d75bfcc1d5e7ed1ee0a086280204a6ea11ac28e). * This patch **passes all tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class Logarithm(left: Expression, right: 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111365733 Merged build finished. Test PASSed. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111337952 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111337944 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111338165 [Test build #34743 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34743/consoleFull) for PR 6725 at commit [`5b39c02`](https://github.com/apache/spark/commit/5b39c0205c0d7043f7d058bf304f1cff0ff451d9). --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111339206 [Test build #34743 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34743/console) for PR 6725 at commit [`5b39c02`](https://github.com/apache/spark/commit/5b39c0205c0d7043f7d058bf304f1cff0ff451d9). * This patch **fails Scala style tests**. * This patch merges cleanly. * This patch adds the following public classes _(experimental)_: * `case class Logarithm(left: Expression, right: 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111339218 Merged build finished. Test 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111340186 Merged build started. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request: [SPARK-8218][SQL] Add binary log math function
Github user AmplabJenkins commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111340179 Merged build triggered. --- 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 infrastruct...@apache.org or file a JIRA ticket with INFRA. --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
