You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by dongjoon-hyun <gi...@git.apache.org> on 2016/04/14 03:09:00 UTC
[GitHub] spark pull request: [SPARK-14614] Add `bround` function
GitHub user dongjoon-hyun opened a pull request:
https://github.com/apache/spark/pull/12376
[SPARK-14614] Add `bround` function
## What changes were proposed in this pull request?
This PR aims to add `bound` function (aka Banker's round) by extending current `round` implementation. [Hive supports `bround` since 1.3.0.](https://cwiki.apache.org/confluence/display/Hive/LanguageManual+UDF)
**Hive (1.3 ~ 2.0)**
```
hive> select round(2.5), bround(2.5);
OK
3.0 2.0
```
**After this PR**
```scala
scala> sql("select round(2.5), bround(2.5)").head
res0: org.apache.spark.sql.Row = [3,2]
```
## How was this patch tested?
Pass the Jenkins tests (with extended tests).
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/dongjoon-hyun/spark SPARK-14614
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12376.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #12376
----
commit 2950339c7a3cde53823f9d5fcafdbbe5b27f9549
Author: Dongjoon Hyun <do...@apache.org>
Date: 2016-04-13T23:08:14Z
[SPARK-14614] Add bround function
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209796006
**[Test build #55805 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55805/consoleFull)** for PR 12376 at commit [`2b004c3`](https://github.com/apache/spark/commit/2b004c3d5e91dec573c3eb94ef877a7dbff82c7e).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209803560
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55799/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209827849
**[Test build #55805 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55805/consoleFull)** for PR 12376 at commit [`2b004c3`](https://github.com/apache/spark/commit/2b004c3d5e91dec573c3eb94ef877a7dbff82c7e).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `abstract class RoundBase(child: Expression, scale: Expression,`
* `case class Round(child: Expression, scale: Expression)`
* `case class BRound(child: Expression, scale: Expression)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210103610
Yes, I am also +1 for a native implementation over using Hive, but my question is more whether we want `bround` in Spark's SQL dialect or whether there is another or standard syntax for accessing bankers' rounding that we should consider. I'm just not sure what our strategy is for adopting non-standardized SQL-ism, whether we are committed to doing whatever Hive does or whether we have some other considerations.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210552825
@dongjoon-hyun Following Hive's lead is definitely one option. I don't know whether it is the right option or whether any strategic decision has been made about how we will handle non-standard SQL functionality in Spark 2.0 -- particularly as we gain independence from Hive's implementation, we could also separate more easily from Hive's interface.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209803557
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614] Add `bround` function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209727087
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55778/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209779631
**[Test build #55799 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55799/consoleFull)** for PR 12376 at commit [`7198a4a`](https://github.com/apache/spark/commit/7198a4a5cf817ef6391ea4fadbd4ff3a3b54bbe0).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by davies <gi...@git.apache.org>.
Github user davies commented on a diff in the pull request:
https://github.com/apache/spark/pull/12376#discussion_r59772070
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1777,6 +1777,23 @@ object functions {
def round(e: Column, scale: Int): Column = withExpr { Round(e.expr, Literal(scale)) }
/**
+ * Returns the value of the column `e` rounded to 0 decimal places with HALF_EVEN round mode.
+ *
+ * @group math_funcs
+ * @since 2.0.0
+ */
+ def bround(e: Column): Column = bround(e, 0)
--- End diff --
Could you add this for Python and R? or create a JIRA for it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209810890
Interesting. While I'm well aware of Bankers' Rounding and the inconsistent implementations of rounding in various SQL engines, I hadn't run into the bround() function before. Searching now, I find that it is also in SQL Server, but it looks to me like this is another bit of rounding functionality that is not standardized across SQL implementations. Do you know any different?
While having more than one rounding strategy available within Spark SQL can be important for interoperating with other systems, I'm not sure if we have decided to always follow HQL, particularly as Spark SQL becomes less directly dependent on Hive in Spark 2.0. @marmbrus ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210550372
The above is my opinion about your second question.
For the first question, we already got three +1 for adding `bround`.
(including yours, thank you.)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209828163
Hi, @markhamstra ! Thank you for commenting.
I agree with your viewpoint. So, this PR has a meaning to add just a function, `bround`; not a HQL language level meaning. In terms of semantics, this is the same implementation with Hive. The following is [Hive code from the Hive master branch](https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/udf/generic/RoundUtils.java).
```java
public static double bround(double input, int scale) {
if (Double.isNaN(input) || Double.isInfinite(input)) {
return input;
}
return BigDecimal.valueOf(input).setScale(scale, RoundingMode.HALF_EVEN).doubleValue();
}
```
By the way, for the last issue, I think in a different way.
Since Spark SQL becomes less directly dependent on Hive, we need to provide this as a Spark SQL function.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12376#discussion_r59776011
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1777,6 +1777,23 @@ object functions {
def round(e: Column, scale: Int): Column = withExpr { Round(e.expr, Literal(scale)) }
/**
+ * Returns the value of the column `e` rounded to 0 decimal places with HALF_EVEN round mode.
+ *
+ * @group math_funcs
+ * @since 2.0.0
+ */
+ def bround(e: Column): Column = bround(e, 0)
--- End diff --
Thank you. I'll fill it soon.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210101241
LGTM
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210109615
Thank you, @marmbrus , @davies , @markhamstra .
@markhamstra . I really appreciate your attention and understand what your concern here correctly, but don't you think it's a little beyond the scope of this kind of tiny PR. I think the topic of your question is related too many things not only this. What about sending an email to dev email list?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614] Add `bround` function
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209711408
**[Test build #55774 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55774/consoleFull)** for PR 12376 at commit [`2950339`](https://github.com/apache/spark/commit/2950339c7a3cde53823f9d5fcafdbbe5b27f9549).
* This patch **fails Scala style tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `abstract class RoundBase(child: Expression, scale: Expression,`
* `case class Round(child: Expression, scale: Expression)`
* `case class BRound(child: Expression, scale: Expression)`
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210550801
By the way, is Spark heading to some SQL Standard?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12376
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209803247
**[Test build #55799 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55799/consoleFull)** for PR 12376 at commit [`7198a4a`](https://github.com/apache/spark/commit/7198a4a5cf817ef6391ea4fadbd4ff3a3b54bbe0).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210558519
Sure. In this year, Spark seems to able to remove Hive code. I agree that Spark is better than Hive and we can do more. But, in terms of Hive compatibility, Spark had better have same semantics with Hive for a few years? In this PR, I mean `bround` semantics, not other syntax.
I'm wondering whether we can be proud of that we have superior `bround` semantics.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on a diff in the pull request:
https://github.com/apache/spark/pull/12376#discussion_r59774717
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1777,6 +1777,23 @@ object functions {
def round(e: Column, scale: Int): Column = withExpr { Round(e.expr, Literal(scale)) }
/**
+ * Returns the value of the column `e` rounded to 0 decimal places with HALF_EVEN round mode.
+ *
+ * @group math_funcs
+ * @since 2.0.0
+ */
+ def bround(e: Column): Column = bround(e, 0)
--- End diff --
No, please don't do JIRAs that way. A JIRA that just refers to a PR (or a PR description that just refers to a JIRA number) is nearly pointless and very annoying. Always include a description in the JIRA motivating *why* a change is needed or desired, and a description of *what* was changed in the PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614] Add `bround` function
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209713660
**[Test build #55778 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55778/consoleFull)** for PR 12376 at commit [`93b6a84`](https://github.com/apache/spark/commit/93b6a846b7d01639a9e9e3c7d8399252877b9e6c).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210086456
Hi, @davies .
Could you review this PR, please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12376#discussion_r59774065
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1777,6 +1777,23 @@ object functions {
def round(e: Column, scale: Int): Column = withExpr { Round(e.expr, Literal(scale)) }
/**
+ * Returns the value of the column `e` rounded to 0 decimal places with HALF_EVEN round mode.
+ *
+ * @group math_funcs
+ * @since 2.0.0
+ */
+ def bround(e: Column): Column = bround(e, 0)
--- End diff --
Thank you for review, @davies .
According to your comments, I created [SPARK-14639](https://issues.apache.org/jira/browse/SPARK-14639) for it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614] Add `bround` function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209711416
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/12376#discussion_r59776934
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1777,6 +1777,23 @@ object functions {
def round(e: Column, scale: Int): Column = withExpr { Round(e.expr, Literal(scale)) }
/**
+ * Returns the value of the column `e` rounded to 0 decimal places with HALF_EVEN round mode.
+ *
+ * @group math_funcs
+ * @since 2.0.0
+ */
+ def bround(e: Column): Column = bround(e, 0)
--- End diff --
@markhamstra . I updated it.
Please let me know if you think I need to do more.
Thank you in many ways.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by marmbrus <gi...@git.apache.org>.
Github user marmbrus commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210100953
+1 to native implementations of hive udfs so we can continue to minimize our dependence.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210549260
Hi, @markhamstra .
If we choose one of `bround` variances, I think we had better choose the one in Hive.
How do you think about that?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-210780699
Hi, @marmbrus , @davies , @markhamstra .
I'm not sure what to do next for this PR. If I am supposed to do something, please let me know.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614] Add `bround` function
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209726678
**[Test build #55778 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55778/consoleFull)** for PR 12376 at commit [`93b6a84`](https://github.com/apache/spark/commit/93b6a846b7d01639a9e9e3c7d8399252877b9e6c).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by markhamstra <gi...@git.apache.org>.
Github user markhamstra commented on a diff in the pull request:
https://github.com/apache/spark/pull/12376#discussion_r59777405
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/functions.scala ---
@@ -1777,6 +1777,23 @@ object functions {
def round(e: Column, scale: Int): Column = withExpr { Round(e.expr, Literal(scale)) }
/**
+ * Returns the value of the column `e` rounded to 0 decimal places with HALF_EVEN round mode.
+ *
+ * @group math_funcs
+ * @since 2.0.0
+ */
+ def bround(e: Column): Column = bround(e, 0)
--- End diff --
Thanks, that's enough. A small, simple issue like this doesn't require a lot of description, but there still should be some.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614] Add `bround` function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209727083
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-211488882
Until now, I cannot find any reason to pursuit other `bround` implementation.
In addition, I think the current implementation of this PR is good for maintainability since it's consistent with existing Spark codebase.
So, could anyone merge this PR please?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614] Add `bround` function
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209710774
**[Test build #55774 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55774/consoleFull)** for PR 12376 at commit [`2950339`](https://github.com/apache/spark/commit/2950339c7a3cde53823f9d5fcafdbbe5b27f9549).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by davies <gi...@git.apache.org>.
Github user davies commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-211495835
Merging this into master, thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209828128
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55805/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614] Add `bround` function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209711418
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55774/
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-14614][SQL] Add `bround` function
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-211508085
Thank you so much, @davies !
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [WIP][SPARK-14614][SQL] Add `bround` function
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12376#issuecomment-209828121
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org