[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 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 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 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-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 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-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 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 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 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-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 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 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 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 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-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-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-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 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) => s"java.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 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 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 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-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 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 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 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-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-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-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 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 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 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 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 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_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-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 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 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_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 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_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 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-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 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 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_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 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 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_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 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 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 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_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 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 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_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_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_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 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 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_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 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 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 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 SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111340400 [Test build #34745 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34745/consoleFull) for PR 6725 at commit [`3d75bfc`](https://github.com/apache/spark/commit/3d75bfcc1d5e7ed1ee0a086280204a6ea11ac28e). --- 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
[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 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 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 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 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 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_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 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 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_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 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 SparkQA commented on the pull request: https://github.com/apache/spark/pull/6725#issuecomment-111256770 [Test build #34712 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/34712/consoleFull) for PR 6725 at commit [`23c54a3`](https://github.com/apache/spark/commit/23c54a381f8a3e062982cc44bf6384c51ec5fd93). --- 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-111256249 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-111256272 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-111253828 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