You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by huaxingao <gi...@git.apache.org> on 2018/05/07 07:05:56 UTC

[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

GitHub user huaxingao opened a pull request:

    https://github.com/apache/spark/pull/21255

    [SPARK-24186][SparR][SQL]change reverse and concat to collection functions in R

    ## What changes were proposed in this pull request?
    
    reverse and concat are already in functions.R as column string functions. Since now these two functions are categorized  as collection functions in scala and python, we will do the same in R. 
    
    ## How was this patch tested?
    
    Add test in test_sparkSQL.R


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/huaxingao/spark spark-24186

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/21255.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 #21255
    
----
commit 3985285089673e42a85a5d1ba3cd7419a6948909
Author: Huaxin Gao <hu...@...>
Date:   2018-05-07T06:47:34Z

    [SPARK-24186][SparR][SQL]add array reverse and concat functions to R

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186610290
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -218,6 +219,8 @@ NULL
     #' head(select(tmp3, map_keys(tmp3$v3)))
     #' head(select(tmp3, map_values(tmp3$v3)))
     #' head(select(tmp3, element_at(tmp3$v3, "Valiant")))}
    --- End diff --
    
    The right `}` for this `dontrun` block is wrongly put here.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90376/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2987/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90352 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90352/testReport)** for PR 21255 at commit [`64ba432`](https://github.com/apache/spark/commit/64ba432f7d854a96e70675ded4fdaa705ad01a06).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90304/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90352 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90352/testReport)** for PR 21255 at commit [`64ba432`](https://github.com/apache/spark/commit/64ba432f7d854a96e70675ded4fdaa705ad01a06).
     * This patch **fails SparkR unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186633965
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -1253,19 +1256,6 @@ setMethod("quarter",
                 column(jc)
               })
     
    -#' @details
    -#' \code{reverse}: Reverses the string column and returns it as a new string column.
    -#'
    -#' @rdname column_string_functions
    -#' @aliases reverse reverse,Column-method
    -#' @note reverse since 1.5.0
    -setMethod("reverse",
    -          signature(x = "Column"),
    -          function(x) {
    -            jc <- callJStatic("org.apache.spark.sql.functions", "reverse", x@jc)
    -            column(jc)
    -          })
    -
    --- End diff --
    
    right, but that's not strictly where collection functions are.
    I'm not saying we need to re-sort/group all functions - that will be thousands of lines we need to change


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][R][SQL]change reverse and concat to...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186942863
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -2047,17 +2049,15 @@ setMethod("countDistinct",
     #' \code{concat}: Concatenates multiple input columns together into a single column.
     #' If all inputs are binary, concat returns an output as binary. Otherwise, it returns as string.
     #'
    -#' @rdname column_string_functions
    +#' @rdname column_collection_functions
     #' @aliases concat concat,Column-method
     #' @examples
     #'
     #' \dontrun{
     #' # concatenate strings
     #' tmp <- mutate(df, s1 = concat(df$Class, df$Sex),
     #'                   s2 = concat(df$Class, df$Sex, df$Age),
    --- End diff --
    
    for background, we want to keep example runnable as much as possible..


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90353 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90353/testReport)** for PR 21255 at commit [`6b36d79`](https://github.com/apache/spark/commit/6b36d7999feb002655666700061f48448b97501b).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90376 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90376/testReport)** for PR 21255 at commit [`b7b50b6`](https://github.com/apache/spark/commit/b7b50b6ad11569c8e2c77f300a2c8cdd7064c959).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][R][SQL]change reverse and concat to...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186942776
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -2047,17 +2049,15 @@ setMethod("countDistinct",
     #' \code{concat}: Concatenates multiple input columns together into a single column.
     #' If all inputs are binary, concat returns an output as binary. Otherwise, it returns as string.
     #'
    -#' @rdname column_string_functions
    +#' @rdname column_collection_functions
     #' @aliases concat concat,Column-method
     #' @examples
     #'
     #' \dontrun{
     #' # concatenate strings
     #' tmp <- mutate(df, s1 = concat(df$Class, df$Sex),
     #'                   s2 = concat(df$Class, df$Sex, df$Age),
    --- End diff --
    
    here `df` is referencing https://github.com/huaxingao/spark/blob/b7b50b6ad11569c8e2c77f300a2c8cdd7064c959/R/pkg/R/functions.R#L141
    
    since rdname is changed, df no longer work here. since you have added an example (perhaps add more? or just add a simple one like the ones here) https://github.com/apache/spark/pull/21255/files#diff-d97f9adc2dcac0703568c799ff106987R222 let's remove the whole example block L 2054-2061 here, if we add one like `concat(df$Class, df$Sex, df$Age, df$Class)` (change to valid column names) to after L 222?



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][R][SQL]change reverse and concat to...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r187515108
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -1256,7 +1259,7 @@ setMethod("quarter",
     #' @details
     #' \code{reverse}: Reverses the string column and returns it as a new string column.
    --- End diff --
    
    Don't we need to update this doc that adds the fact it supports array now?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3046/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90493/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90352/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186604950
  
    --- Diff: R/pkg/tests/fulltests/test_sparkSQL.R ---
    @@ -1748,6 +1748,14 @@ test_that("string operators", {
         collect(select(df5, repeat_string(df5$a, -1)))[1, 1],
         ""
       )
    +
    +  l6 <- list(list(a = "abc"))
    +  df6 <- createDataFrame(l6)
    +  df7 <- select(df6, reverse(df6$a))
    --- End diff --
    
    `df7` is not used below?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90376 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90376/testReport)** for PR 21255 at commit [`b7b50b6`](https://github.com/apache/spark/commit/b7b50b6ad11569c8e2c77f300a2c8cdd7064c959).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    PR title `[SparR]` -> `[SparkR]` or `[R]`. FWIW, I use `[R]` or `[PYTHON]` since that's what you see if you read the contribution guide closely.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90353 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90353/testReport)** for PR 21255 at commit [`6b36d79`](https://github.com/apache/spark/commit/6b36d7999feb002655666700061f48448b97501b).
     * This patch **fails SparkR unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90299 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90299/testReport)** for PR 21255 at commit [`3985285`](https://github.com/apache/spark/commit/3985285089673e42a85a5d1ba3cd7419a6948909).
     * This patch **fails to build**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186368498
  
    --- Diff: R/pkg/tests/fulltests/test_sparkSQL.R ---
    @@ -1502,12 +1502,21 @@ test_that("column functions", {
       result <- collect(select(df, sort_array(df[[1]])))[[1]]
       expect_equal(result, list(list(1L, 2L, 3L), list(4L, 5L, 6L)))
     
    -  # Test flattern
    +  result <- collect(select(df, reverse(df[[1]])))[[1]]
    --- End diff --
    
    Seems we don't have test for `reverse` for string. Can you add one for it too?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90304 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90304/testReport)** for PR 21255 at commit [`3985285`](https://github.com/apache/spark/commit/3985285089673e42a85a5d1ba3cd7419a6948909).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Yes
    
    ________________________________
    From: Hyukjin Kwon <no...@github.com>
    Sent: Thursday, May 10, 2018 9:16:28 AM
    To: apache/spark
    Cc: Felix Cheung; Mention
    Subject: Re: [apache/spark] [SPARK-24186][R][SQL]change reverse and concat to collection functions in R (#21255)
    
    
    @HyukjinKwon commented on this pull request.
    
    ________________________________
    
    In R/pkg/tests/fulltests/test_sparkSQL.R<https://github.com/apache/spark/pull/21255#discussion_r187380148>:
    
    > @@ -1502,12 +1502,25 @@ test_that("column functions", {
       result <- collect(select(df, sort_array(df[[1]])))[[1]]
       expect_equal(result, list(list(1L, 2L, 3L), list(4L, 5L, 6L)))
    
    -  # Test flattern
    +  result <- collect(select(df, reverse(df[[1]])))[[1]]
    +  expect_equal(result, list(list(3L, 2L, 1L), list(4L, 5L, 6L)))
    +
    +  df2 <- createDataFrame(list(list("abc")))
    +  result <- collect(select(df2, reverse(df2[[1]])))[[1]]
    +  expect_equal(result, "cba")
    +
    +  # Test flattern()
    
    
    flattern -> flatten.
    
    —
    You are receiving this because you were mentioned.
    Reply to this email directly, view it on GitHub<https://github.com/apache/spark/pull/21255#pullrequestreview-119149584>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AIjc-9PLjYLZAi6YHkE9OSrg0zVnOHrjks5txGfcgaJpZM4T0f1k>.



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186624766
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -1253,19 +1256,6 @@ setMethod("quarter",
                 column(jc)
               })
     
    -#' @details
    -#' \code{reverse}: Reverses the string column and returns it as a new string column.
    -#'
    -#' @rdname column_string_functions
    -#' @aliases reverse reverse,Column-method
    -#' @note reverse since 1.5.0
    -setMethod("reverse",
    -          signature(x = "Column"),
    -          function(x) {
    -            jc <- callJStatic("org.apache.spark.sql.functions", "reverse", x@jc)
    -            column(jc)
    -          })
    -
    --- End diff --
    
    so these func are not actually ordered or grouped - my preference would be not to move then around unless we have to?



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][R][SQL]change reverse and concat to...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r187515152
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -2047,18 +2050,8 @@ setMethod("countDistinct",
     #' \code{concat}: Concatenates multiple input columns together into a single column.
     #' If all inputs are binary, concat returns an output as binary. Otherwise, it returns as string.
    --- End diff --
    
    ditto.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186624379
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -209,6 +209,7 @@ NULL
     #' head(select(tmp, array_max(tmp$v1), array_min(tmp$v1)))
     #' head(select(tmp, array_position(tmp$v1, 21)))
     #' head(select(tmp, flatten(tmp$v1)))
    +#' head(select(tmp, reverse(tmp$v1)))
    --- End diff --
    
    let's merge into an existing line - we don't need one line per function; don't want these too be too long each line but also not too many lines either


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][R][SQL]change reverse and concat to...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r187238518
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -219,7 +219,8 @@ NULL
     #' head(select(tmp3, map_values(tmp3$v3)))
     #' head(select(tmp3, element_at(tmp3$v3, "Valiant")))
     #' tmp4 <- mutate(df, v4 = create_array(df$mpg, df$cyl), v5 = create_array(df$hp))
    -#' head(select(tmp4, concat(tmp4$v4, tmp4$v5)))}
    +#' head(select(tmp4, concat(tmp4$v4, tmp4$v5)))
    +#' concat(df$mpg, df$cyl, df$hp)}
    --- End diff --
    
    I'd perhaps do this as
    ```
    tmp5 <- mutate(df, s1 = concat(df$mpg, df$cyl, df$hp)
    head(tmp5)
    ```
    
    or
    
    ```
    head(mutate(df, s1 = concat(df$mpg, df$cyl, df$hp))
    ```
    
    btw, aren't these numeric columns? does that work with concat?



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186369103
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -209,6 +209,7 @@ NULL
     #' head(select(tmp, array_max(tmp$v1), array_min(tmp$v1)))
     #' head(select(tmp, array_position(tmp$v1, 21)))
     #' head(select(tmp, flatten(tmp$v1)))
    +#' head(select(tmp, reverse(tmp$v1)))
    --- End diff --
    
    Also add `concat` here?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90470/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    also cc @felixcheung.



---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186635312
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -1253,19 +1256,6 @@ setMethod("quarter",
                 column(jc)
               })
     
    -#' @details
    -#' \code{reverse}: Reverses the string column and returns it as a new string column.
    -#'
    -#' @rdname column_string_functions
    -#' @aliases reverse reverse,Column-method
    -#' @note reverse since 1.5.0
    -setMethod("reverse",
    -          signature(x = "Column"),
    -          function(x) {
    -            jc <- callJStatic("org.apache.spark.sql.functions", "reverse", x@jc)
    -            column(jc)
    -          })
    -
    --- End diff --
    
    I will put them back to the original places and add back the examples. Thanks!


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90493 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90493/testReport)** for PR 21255 at commit [`675726c`](https://github.com/apache/spark/commit/675726c326f2dd201e94f0def0b395befb87d303).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][R][SQL]change reverse and concat to...

Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao closed the pull request at:

    https://github.com/apache/spark/pull/21255


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3024/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90353/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    retest this please


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90426 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90426/testReport)** for PR 21255 at commit [`b50d2c8`](https://github.com/apache/spark/commit/b50d2c88b5feb5c61572b0eb2fa1c93773ae3919).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90426 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90426/testReport)** for PR 21255 at commit [`b50d2c8`](https://github.com/apache/spark/commit/b50d2c88b5feb5c61572b0eb2fa1c93773ae3919).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90304 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90304/testReport)** for PR 21255 at commit [`3985285`](https://github.com/apache/spark/commit/3985285089673e42a85a5d1ba3cd7419a6948909).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][R][SQL]change reverse and concat to...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r187380148
  
    --- Diff: R/pkg/tests/fulltests/test_sparkSQL.R ---
    @@ -1502,12 +1502,25 @@ test_that("column functions", {
       result <- collect(select(df, sort_array(df[[1]])))[[1]]
       expect_equal(result, list(list(1L, 2L, 3L), list(4L, 5L, 6L)))
     
    -  # Test flattern
    +  result <- collect(select(df, reverse(df[[1]])))[[1]]
    +  expect_equal(result, list(list(3L, 2L, 1L), list(4L, 5L, 6L)))
    +
    +  df2 <- createDataFrame(list(list("abc")))
    +  result <- collect(select(df2, reverse(df2[[1]])))[[1]]
    +  expect_equal(result, "cba")
    +
    +  # Test flattern()
    --- End diff --
    
    `flattern` -> `flatten`. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3084/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90493 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90493/testReport)** for PR 21255 at commit [`675726c`](https://github.com/apache/spark/commit/675726c326f2dd201e94f0def0b395befb87d303).
     * This patch **fails due to an unknown error code, -9**.
     * This patch **does not merge cleanly**.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186612845
  
    --- Diff: R/pkg/tests/fulltests/test_sparkSQL.R ---
    @@ -1739,6 +1748,13 @@ test_that("string operators", {
         collect(select(df5, repeat_string(df5$a, -1)))[1, 1],
         ""
       )
    +
    +  l6 <- list(list(a = "abc"))
    +  df6 <- createDataFrame(l6)
    --- End diff --
    
    I would combine those two lines too.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186624915
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -2043,34 +2033,6 @@ setMethod("countDistinct",
                 column(jc)
               })
     
    -#' @details
    -#' \code{concat}: Concatenates multiple input columns together into a single column.
    -#' If all inputs are binary, concat returns an output as binary. Otherwise, it returns as string.
    -#'
    -#' @rdname column_string_functions
    -#' @aliases concat concat,Column-method
    -#' @examples
    -#'
    -#' \dontrun{
    -#' # concatenate strings
    -#' tmp <- mutate(df, s1 = concat(df$Class, df$Sex),
    -#'                   s2 = concat(df$Class, df$Sex, df$Age),
    -#'                   s3 = concat(df$Class, df$Sex, df$Age, df$Class),
    -#'                   s4 = concat_ws("_", df$Class, df$Sex),
    -#'                   s5 = concat_ws("+", df$Class, df$Sex, df$Age, df$Survived))
    --- End diff --
    
    also what happen to all other examples?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90470 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90470/testReport)** for PR 21255 at commit [`176d314`](https://github.com/apache/spark/commit/176d3147132f224b53649217e5687e5851e0f429).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3126/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2982/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90470 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90470/testReport)** for PR 21255 at commit [`176d314`](https://github.com/apache/spark/commit/176d3147132f224b53649217e5687e5851e0f429).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186612820
  
    --- Diff: R/pkg/tests/fulltests/test_sparkSQL.R ---
    @@ -1739,6 +1748,13 @@ test_that("string operators", {
         collect(select(df5, repeat_string(df5$a, -1)))[1, 1],
         ""
       )
    +
    +  l6 <- list(list(a = "abc"))
    +  df6 <- createDataFrame(l6)
    +  expect_equal(
    +    collect(select(df6, reverse(df6$a)))[1, 1],
    +    "cba"
    +  )
    --- End diff --
    
    Let's make this inlined while we are here. I would also put this test around 1505L above.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    **[Test build #90299 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90299/testReport)** for PR 21255 at commit [`3985285`](https://github.com/apache/spark/commit/3985285089673e42a85a5d1ba3cd7419a6948909).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186631550
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -2043,34 +2033,6 @@ setMethod("countDistinct",
                 column(jc)
               })
     
    -#' @details
    -#' \code{concat}: Concatenates multiple input columns together into a single column.
    -#' If all inputs are binary, concat returns an output as binary. Otherwise, it returns as string.
    -#'
    -#' @rdname column_string_functions
    -#' @aliases concat concat,Column-method
    -#' @examples
    -#'
    -#' \dontrun{
    -#' # concatenate strings
    -#' tmp <- mutate(df, s1 = concat(df$Class, df$Sex),
    -#'                   s2 = concat(df$Class, df$Sex, df$Age),
    -#'                   s3 = concat(df$Class, df$Sex, df$Age, df$Class),
    -#'                   s4 = concat_ws("_", df$Class, df$Sex),
    -#'                   s5 = concat_ws("+", df$Class, df$Sex, df$Age, df$Survived))
    --- End diff --
    
    Since now I put concat in the Collection functions section, I am not sure if I should keep the column_string_functions examples there. I will add them back if you prefer to keep these examples. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186630904
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -1253,19 +1256,6 @@ setMethod("quarter",
                 column(jc)
               })
     
    -#' @details
    -#' \code{reverse}: Reverses the string column and returns it as a new string column.
    -#'
    -#' @rdname column_string_functions
    -#' @aliases reverse reverse,Column-method
    -#' @note reverse since 1.5.0
    -setMethod("reverse",
    -          signature(x = "Column"),
    -          function(x) {
    -            jc <- callJStatic("org.apache.spark.sql.functions", "reverse", x@jc)
    -            column(jc)
    -          })
    -
    --- End diff --
    
    @felixcheung Thanks for your comment. The reason I moved reverse and concat around is that I want to put them under line 2943 for Collection functions. 
    ###################### Collection functions######################


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    @felixcheung @HyukjinKwon @viirya 
    I am thinking of closing this one and open a new PR if no objection. It's messy to resolve the conflicts because I have quite a few patches. Sorry for my carelessness and for any inconvenience.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][R][SQL]change reverse and concat to...

Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r187262627
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -219,7 +219,8 @@ NULL
     #' head(select(tmp3, map_values(tmp3$v3)))
     #' head(select(tmp3, element_at(tmp3$v3, "Valiant")))
     #' tmp4 <- mutate(df, v4 = create_array(df$mpg, df$cyl), v5 = create_array(df$hp))
    -#' head(select(tmp4, concat(tmp4$v4, tmp4$v5)))}
    +#' head(select(tmp4, concat(tmp4$v4, tmp4$v5)))
    +#' concat(df$mpg, df$cyl, df$hp)}
    --- End diff --
    
    @felixcheung 
    Yes, the numeric columns work OK with concat. I tested and it has the following
    +--------------------+
    |concat(mpg, cyl, hp)|
    +--------------------+
    |        21.06.0110.0|
    |        21.06.0110.0|
    Is it OK with you to have the example as ```head(select(tmp, concat(df$mpg, df$cyl, df$hp)))``` ? Somehow, the test failed with ```head(mutate(df, s1 = concat(df$mpg, df$cyl, df$hp))```


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by felixcheung <gi...@git.apache.org>.
Github user felixcheung commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186634471
  
    --- Diff: R/pkg/R/functions.R ---
    @@ -2043,34 +2033,6 @@ setMethod("countDistinct",
                 column(jc)
               })
     
    -#' @details
    -#' \code{concat}: Concatenates multiple input columns together into a single column.
    -#' If all inputs are binary, concat returns an output as binary. Otherwise, it returns as string.
    -#'
    -#' @rdname column_string_functions
    -#' @aliases concat concat,Column-method
    -#' @examples
    -#'
    -#' \dontrun{
    -#' # concatenate strings
    -#' tmp <- mutate(df, s1 = concat(df$Class, df$Sex),
    -#'                   s2 = concat(df$Class, df$Sex, df$Age),
    -#'                   s3 = concat(df$Class, df$Sex, df$Age, df$Class),
    -#'                   s4 = concat_ws("_", df$Class, df$Sex),
    -#'                   s5 = concat_ws("+", df$Class, df$Sex, df$Age, df$Survived))
    --- End diff --
    
    yes, let's not lose example.
    we should add concat to the column_collection_functions
    and concat_ws to column_string_functions


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3110/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90299/
    Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/3023/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    @huaxingao, that's absolutely fine.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark pull request #21255: [SPARK-24186][SparR][SQL]change reverse and conca...

Posted by huaxingao <gi...@git.apache.org>.
Github user huaxingao commented on a diff in the pull request:

    https://github.com/apache/spark/pull/21255#discussion_r186604721
  
    --- Diff: R/pkg/tests/fulltests/test_sparkSQL.R ---
    @@ -1502,12 +1502,21 @@ test_that("column functions", {
       result <- collect(select(df, sort_array(df[[1]])))[[1]]
       expect_equal(result, list(list(1L, 2L, 3L), list(4L, 5L, 6L)))
     
    -  # Test flattern
    +  result <- collect(select(df, reverse(df[[1]])))[[1]]
    --- End diff --
    
    @viirya Thank for your comments. I will make changes. 


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][SparR][SQL]change reverse and concat to co...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Merged build finished. Test FAILed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] spark issue #21255: [SPARK-24186][R][SQL]change reverse and concat to collec...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21255
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/90426/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org