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 2017/01/13 19:06:35 UTC

[GitHub] spark pull request #16579: [SPARK-19218][SQL] SET command should show a resu...

GitHub user dongjoon-hyun opened a pull request:

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

    [SPARK-19218][SQL] SET command should show a result sorted by key

    ## What changes were proposed in this pull request?
    
    Currently, `SET` command shows unsorted result. We had better show a sorted result for UX. Also, this is compatible with Hive.
    
    **BEFORE**
    ```
    scala> sql("set").show(false)
    ...
    |spark.driver.host              |10.22.16.140                                                                                                                                 |
    |spark.driver.port              |63893                                                                                                                                        |
    |spark.repl.class.uri           |spark://10.22.16.140:63893/classes                                                                                                           |
    ...
    |spark.app.name                 |Spark shell                                                                                                                                  |
    |spark.driver.memory            |4G                                                                                                                                           |
    |spark.executor.id              |driver                                                                                                                                       |
    |spark.submit.deployMode        |client                                                                                                                                       |
    |spark.master                   |local[*]                                                                                                                                     |
    |spark.home                     |/Users/dhyun/spark                                                                                                                           |
    |spark.sql.catalogImplementation|hive                                                                                                                                         |
    |spark.app.id                   |local-1484333618945                                                                                                                          |
    ```
    
    **AFTER**
    ```
    scala> sql("set").show(false)
    ...
    |spark.app.id                   |local-1484333925649                                                                                                                          |
    |spark.app.name                 |Spark shell                                                                                                                                  |
    |spark.driver.host              |10.22.16.140                                                                                                                                 |
    |spark.driver.memory            |4G                                                                                                                                           |
    |spark.driver.port              |64994                                                                                                                                        |
    |spark.executor.id              |driver                                                                                                                                       |
    |spark.jars                     |                                                                                                                                             |
    |spark.master                   |local[*]                                                                                                                                     |
    |spark.repl.class.uri           |spark://10.22.16.140:64994/classes                                                                                                           |
    |spark.sql.catalogImplementation|hive                                                                                                                                         |
    |spark.submit.deployMode        |client                                                                                                                                       |
    ```
    
    ## How was this patch tested?
    
    Jenkins with a new test case.

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

    $ git pull https://github.com/dongjoon-hyun/spark SPARK-19218

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

    https://github.com/apache/spark/pull/16579.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 #16579
    
----
commit 052a2f489c8e7e4889a64a673fdce5df2422f2cb
Author: Dongjoon Hyun <do...@apache.org>
Date:   2017-01-05T00:28:16Z

    [SPARK-19218][SQL] SET command should show a sorted result

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71815 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71815/testReport)** for PR 16579 at commit [`528b0fd`](https://github.com/apache/spark/commit/528b0fd1bd8a685f14d708d1a47570924835e569).
     * This patch passes all 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    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


[GitHub] spark issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    The failure does not happen local pc, but it always happens in Jenkins.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    Retest this 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71344 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71344/testReport)** for PR 16579 at commit [`332935b`](https://github.com/apache/spark/commit/332935b322733d772733a52871e2b6b0a67af633).
     * 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71348 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71348/testReport)** for PR 16579 at commit [`9576d51`](https://github.com/apache/spark/commit/9576d514617a9db29340d37789dfde7e05ff98d3).
     * 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    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


[GitHub] spark issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71821/
    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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Interesting. The only failure was a new test case.
    ```
    [info] - SET commands should return a list sorted by key *** FAILED *** (18 milliseconds)
    [info]   java.lang.RuntimeException: Error while decoding: java.lang.NullPointerException
    [info] createexternalrow(input[0, string, false].toString, input[1, string, false].toString, input[2, string, false].toString, StructField(key,StringType,false), StructField(value,StringType,false), StructField(meaning,StringType,false))
    [info] :- input[0, string, false].toString
    [info] :  +- input[0, string, false]
    [info] :- input[1, string, false].toString
    [info] :  +- input[1, string, false]
    [info] +- input[2, string, false].toString
    [info]    +- input[2, string, false]
    [info]   at org.apache.spark.sql.catalyst.encoders.ExpressionEncoder.fromRow(ExpressionEncoder.scala:303)
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71525 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71525/testReport)** for PR 16579 at commit [`f767e11`](https://github.com/apache/spark/commit/f767e119d0f25d12389446552ab2ebc04519fcdf).
     * 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71348/
    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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71352/
    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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Thanks! Merging to master.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97250196
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    :) The point is `the other test cases` are still running.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71809 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71809/testReport)** for PR 16579 at commit [`0214fad`](https://github.com/apache/spark/commit/0214fadae05ac80dcb9662d3953f2708d5f1605d).
     * This patch passes all 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97229801
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,16 +79,17 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.toSeq.sorted.map { case (k, v) => Row(k, v) }
           }
           (keyValueOutput, runFunc)
     
         // Queries all properties along with their default values and docs that are defined in the
         // SQLConf of the sparkSession.
         case Some(("-v", None)) =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.sessionState.conf.getAllDefinedConfs.map { case (key, defaultValue, doc) =>
    -          Row(key, defaultValue, doc)
    +        sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
    +          case (key, defaultValue, doc) =>
    +            Row(key, if (defaultValue == null) "null" else defaultValue, doc)
    --- End diff --
    
    When the default is null, we are using `<undefined>`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71352 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71352/testReport)** for PR 16579 at commit [`9576d51`](https://github.com/apache/spark/commit/9576d514617a9db29340d37789dfde7e05ff98d3).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71822 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71822/testReport)** for PR 16579 at commit [`7879201`](https://github.com/apache/spark/commit/7879201961b0f0caa997c9fe6446c0b1b46124f8).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71822 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71822/testReport)** for PR 16579 at commit [`7879201`](https://github.com/apache/spark/commit/7879201961b0f0caa997c9fe6446c0b1b46124f8).
     * 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 #16579: [SPARK-19218][SQL] SET command should show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r96058727
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,7 +79,7 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq.sortBy(_.getString(0))
    --- End diff --
    
    Is it simpler to sort earlier?  `sparkSession.conf.getAll.toSeq.sorted.map { case (k, v) => Row(k, v) }`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    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


[GitHub] spark issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71525/
    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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71342/
    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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97230236
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,16 +79,17 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.toSeq.sorted.map { case (k, v) => Row(k, v) }
           }
           (keyValueOutput, runFunc)
     
         // Queries all properties along with their default values and docs that are defined in the
         // SQLConf of the sparkSession.
         case Some(("-v", None)) =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.sessionState.conf.getAllDefinedConfs.map { case (key, defaultValue, doc) =>
    -          Row(key, defaultValue, doc)
    +        sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
    +          case (key, defaultValue, doc) =>
    +            Row(key, if (defaultValue == null) "null" else defaultValue, doc)
    --- End diff --
    
    It's easy to change to that, but that seems to be for `Option` type config. 
    Is it better to use the same `<undefined>`?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71517 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71517/testReport)** for PR 16579 at commit [`337d02d`](https://github.com/apache/spark/commit/337d02d3fcdec696eb9e89bcad8094d5b34c1f59).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97230382
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,16 +79,17 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.toSeq.sorted.map { case (k, v) => Row(k, v) }
           }
           (keyValueOutput, runFunc)
     
         // Queries all properties along with their default values and docs that are defined in the
         // SQLConf of the sparkSession.
         case Some(("-v", None)) =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.sessionState.conf.getAllDefinedConfs.map { case (key, defaultValue, doc) =>
    -          Row(key, defaultValue, doc)
    +        sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
    +          case (key, defaultValue, doc) =>
    +            Row(key, if (defaultValue == null) "null" else defaultValue, doc)
    --- End diff --
    
    I think so. We did it [here](https://github.com/dongjoon-hyun/spark/blob/fb578d14c49b895608336bdddd7bff5a36759198/sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala#L116).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97249076
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    oh, i meant that you actually don't need a try {} finally {} here. you don't cache anything.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    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


[GitHub] spark issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    @gatorsmile . I updated with `<undefined>` and added the test case.
    If you can revert the change on `SetCommand.scala`, this test case will fail.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97230429
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,16 +79,17 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.toSeq.sorted.map { case (k, v) => Row(k, v) }
           }
           (keyValueOutput, runFunc)
     
         // Queries all properties along with their default values and docs that are defined in the
         // SQLConf of the sparkSession.
         case Some(("-v", None)) =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.sessionState.conf.getAllDefinedConfs.map { case (key, defaultValue, doc) =>
    -          Row(key, defaultValue, doc)
    +        sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
    +          case (key, defaultValue, doc) =>
    +            Row(key, if (defaultValue == null) "null" else defaultValue, doc)
    --- End diff --
    
    Oh, I see. Thank you. I'll update like 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97249854
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    hmm, when it throws an exception, the whole test fails. does it still matter it interferes other tests or not. :-)
    
    it is harmless to keep this try block, anyway.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71344 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71344/testReport)** for PR 16579 at commit [`332935b`](https://github.com/apache/spark/commit/332935b322733d772733a52871e2b6b0a67af633).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71777/
    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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71809/
    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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Hi, @srowen and @gatorsmile .
    Finally, this PR resolved all issues.
    Could you review this again?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71781/
    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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Retest this 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71344/
    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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    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


[GitHub] spark issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Thank you for approval, @srowen !


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71781 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71781/testReport)** for PR 16579 at commit [`fb578d1`](https://github.com/apache/spark/commit/fb578d14c49b895608336bdddd7bff5a36759198).
     * This patch passes all 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71381/
    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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71342 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71342/testReport)** for PR 16579 at commit [`052a2f4`](https://github.com/apache/spark/commit/052a2f489c8e7e4889a64a673fdce5df2422f2cb).
     * This patch passes all 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71363 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71363/testReport)** for PR 16579 at commit [`5237174`](https://github.com/apache/spark/commit/5237174d488de7c356e75a8eb230f2ef704eac20).
     * 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97239778
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,16 +79,17 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.toSeq.sorted.map { case (k, v) => Row(k, v) }
           }
           (keyValueOutput, runFunc)
     
         // Queries all properties along with their default values and docs that are defined in the
         // SQLConf of the sparkSession.
         case Some(("-v", None)) =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.sessionState.conf.getAllDefinedConfs.map { case (key, defaultValue, doc) =>
    -          Row(key, defaultValue, doc)
    +        sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
    +          case (key, defaultValue, doc) =>
    +            Row(key, if (defaultValue == null) "<undefined>" else defaultValue, doc)
    --- End diff --
    
    Yep. It looks much better.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97250343
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    Maybe, we are confusing on *terms*.
    - You meant the other test *statements*.
    - I meant the other test *cases*


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97209831
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,16 +79,17 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.toSeq.sorted.map { case (k, v) => Row(k, v) }
           }
           (keyValueOutput, runFunc)
     
         // Queries all properties along with their default values and docs that are defined in the
         // SQLConf of the sparkSession.
         case Some(("-v", None)) =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.sessionState.conf.getAllDefinedConfs.map { case (key, defaultValue, doc) =>
    -          Row(key, defaultValue, doc)
    +        sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
    +          case (key, defaultValue, doc) =>
    +            Row(key, if (defaultValue == null) "null" else defaultValue, doc)
    --- End diff --
    
    This is the root cause to raise exceptions during decoding.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71824 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71824/testReport)** for PR 16579 at commit [`7879201`](https://github.com/apache/spark/commit/7879201961b0f0caa997c9fe6446c0b1b46124f8).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Hi, @gatorsmile .
    This is the original PR which has two fixes together now.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71824/
    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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #3535 has started](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3535/testReport)** for PR 16579 at commit [`337d02d`](https://github.com/apache/spark/commit/337d02d3fcdec696eb9e89bcad8094d5b34c1f59).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

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


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71517/
    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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71517 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71517/testReport)** for PR 16579 at commit [`337d02d`](https://github.com/apache/spark/commit/337d02d3fcdec696eb9e89bcad8094d5b34c1f59).
     * 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Retest this 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71352 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71352/testReport)** for PR 16579 at commit [`9576d51`](https://github.com/apache/spark/commit/9576d514617a9db29340d37789dfde7e05ff98d3).
     * 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97249218
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    Ah, I see what you meant. Actually, previously, `SET -v` raises exceptions, so this case use `try` and `catch`. But, as you mentioned, now it's not.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97250587
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    oh, i meant the final Jenkins test result is failed. nvm, i think it is still useful so we can better infer which test causes the failure if we don't interfere other tests.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97240285
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,28 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 Fix SET command to show a result correctly and in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +
    +    // Previsouly, `SET -v` fails with NPE during decoding for null value.
    +    import SQLConf._
    +    SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    val result2 = sql("SET -v").collect()
    +    assert(result2 === result2.sortBy(_.getString(0)))
    +    spark.sessionState.conf.clear()
    --- End diff --
    
    This will not drop the `spark.test`. We need to introduce a function for testing only
    ```Scala
      // For testing only
      private[sql] def unregister(entry: ConfigEntry[_]): Unit = sqlConfEntries.synchronized {
        sqlConfEntries.remove(entry.key)
      }
    ```
    
    Create a separate test case; then call the following code in a finally block: `SQLConf.unregister(confEntry)`
    
    Will be back after a few hours. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Please try it and then we can check whether it makes sense to do it in the same test case or not.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71777 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71777/testReport)** for PR 16579 at commit [`fa4914b`](https://github.com/apache/spark/commit/fa4914bf02f6986e83eeeede8110f884e342b174).
     * This patch passes all 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    LGTM pending test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71809 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71809/testReport)** for PR 16579 at commit [`0214fad`](https://github.com/apache/spark/commit/0214fadae05ac80dcb9662d3953f2708d5f1605d).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97240545
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,28 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 Fix SET command to show a result correctly and in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +
    +    // Previsouly, `SET -v` fails with NPE during decoding for null value.
    +    import SQLConf._
    +    SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    val result2 = sql("SET -v").collect()
    +    assert(result2 === result2.sortBy(_.getString(0)))
    +    spark.sessionState.conf.clear()
    --- End diff --
    
    +1. Thank you, @gatorsmile .


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97250113
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    it is failed. isn't 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97248522
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    +      spark.sessionState.conf.clear()
    +    } finally {
    --- End diff --
    
    nit: try ... finally seems redundant.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71821 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71821/testReport)** for PR 16579 at commit [`7061cd9`](https://github.com/apache/spark/commit/7061cd9ccd5684301efb2c6c6a8b05af36f65417).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97229813
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,16 +79,17 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.toSeq.sorted.map { case (k, v) => Row(k, v) }
           }
           (keyValueOutput, runFunc)
     
         // Queries all properties along with their default values and docs that are defined in the
         // SQLConf of the sparkSession.
         case Some(("-v", None)) =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.sessionState.conf.getAllDefinedConfs.map { case (key, defaultValue, doc) =>
    -          Row(key, defaultValue, doc)
    +        sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
    +          case (key, defaultValue, doc) =>
    +            Row(key, if (defaultValue == null) "null" else defaultValue, doc)
    --- End diff --
    
    When the default is null, we are using `<undefined>`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    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


[GitHub] spark issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Yep!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] SET command should show a resu...

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/16579#discussion_r96065519
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,7 +79,7 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq.sortBy(_.getString(0))
    --- End diff --
    
    Thank you for review, @srowen . Sure, I'll update the PR like 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Thank you, @viirya .
    I noticed that `spark.sessionState.conf.clear()` is useless. I removed 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #3535 has finished](https://amplab.cs.berkeley.edu/jenkins/job/NewSparkPullRequestBuilder/3535/testReport)** for PR 16579 at commit [`337d02d`](https://github.com/apache/spark/commit/337d02d3fcdec696eb9e89bcad8094d5b34c1f59).
     * This patch passes all 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    @dongjoon-hyun what do you think the status is here -- is it possible to fix the tests or still an unknown problem?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71381 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71381/testReport)** for PR 16579 at commit [`337d02d`](https://github.com/apache/spark/commit/337d02d3fcdec696eb9e89bcad8094d5b34c1f59).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    @gatorsmile  Thank you for review and approval, too!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71815 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71815/testReport)** for PR 16579 at commit [`528b0fd`](https://github.com/apache/spark/commit/528b0fd1bd8a685f14d708d1a47570924835e569).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    LGTM pending test


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Retest this 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97239718
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,16 +79,17 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.toSeq.sorted.map { case (k, v) => Row(k, v) }
           }
           (keyValueOutput, runFunc)
     
         // Queries all properties along with their default values and docs that are defined in the
         // SQLConf of the sparkSession.
         case Some(("-v", None)) =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.sessionState.conf.getAllDefinedConfs.map { case (key, defaultValue, doc) =>
    -          Row(key, defaultValue, doc)
    +        sparkSession.sessionState.conf.getAllDefinedConfs.sorted.map {
    +          case (key, defaultValue, doc) =>
    +            Row(key, if (defaultValue == null) "<undefined>" else defaultValue, doc)
    --- End diff --
    
    Let us do it using a more scala way:
    ```
                Row(key, Option(defaultValue).getOrElse("<undefined>"), doc)
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97249644
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    Yes, but we need to clean up `spark.test` in order not to interrupt the other test cases here.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71824 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71824/testReport)** for PR 16579 at commit [`7879201`](https://github.com/apache/spark/commit/7879201961b0f0caa997c9fe6446c0b1b46124f8).
     * This patch passes all 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Thank you for keeping your attention and approval again, @srowen !


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71773/
    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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Yes. It was because the `stringConf` with default `null` is in YARN module. So, when I run a Unit test or `sql` module test. It doesn't meet this.
    
    Yesterday, I reproduced this in my local by add `stringConf` with default `null` in `SQLConf.scala` and fixed this.



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97250719
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    Oh, I understand. 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    The root cause is not inside a newly added code.
    ```
    org.apache.spark.sql.SQLQuerySuite.SET -v test	43 ms	1
    org.apache.spark.sql.SQLQuerySuite.`SET -v` commands should return a list sorted by key
    ```
    
    The current `set -v` implementation seems to have issue according to the error message. I'll make another PR to clarify that.
    ```
    org.apache.spark.SparkException: Job aborted due to stage failure: Task 0 in stage 480.0 failed 1 times, most recent failure: Lost task 0.0 in stage 480.0 (TID 1470, localhost, executor driver): java.lang.NullPointerException  at org.apache.spark.sql.catalyst.expressions.GeneratedClass$GeneratedIterator.processNext(Unknown Source)  at org.apache.spark.sql.execution.BufferedRowIterator.hasNext(BufferedRowIterator.java:43)  at org.apache.spark.sql.execution.WholeStageCodegenExec$$anonfun$8$$anon$1.hasNext(WholeStageCodegenExec.scala:377)  at org.apache.spark.sql.execution.SparkPlan$$anonfun$2.apply(SparkPlan.scala:231)  at org.apache.spark.sql.execution.SparkPlan$$anonfun$2.apply(SparkPlan.scala:225)  at org.apache.spark.rdd.RDD$$anonfun$mapPartitionsInternal$1$$anonfun$apply$25.apply(RDD.scala:826)  at org.apache.spark.rdd.RDD$$anonfun$mapPartitionsInternal$1$$anonfun$apply$25.apply(RDD.scala:826)  at org.apache.spark.rdd.MapPartitionsRDD.compute(MapPartitionsRDD.scala:38)  at org
 .apache.spark.rdd.RDD.computeOrReadCheckpoint(RDD.scala:323)  at org.apache.spark.rdd.RDD.iterator(RDD.scala:287)  at org.apache.spark.scheduler.ResultTask.runTask(ResultTask.scala:88)  at org.apache.spark.scheduler.Task.run(Task.scala:114)  at org.apache.spark.executor.Executor$TaskRunner.run(Executor.scala:313)  at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)  at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)  at java.lang.Thread.run(Thread.java:745)  Driver stacktrace:
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71815/
    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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    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


[GitHub] spark issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71773 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71773/testReport)** for PR 16579 at commit [`fa4914b`](https://github.com/apache/spark/commit/fa4914bf02f6986e83eeeede8110f884e342b174).
     * This patch passes all 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Yes. BTW, to do that, we need to add the test case in `SET -v` unit test. Is it okay?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    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


[GitHub] spark issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71348 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71348/testReport)** for PR 16579 at commit [`9576d51`](https://github.com/apache/spark/commit/9576d514617a9db29340d37789dfde7e05ff98d3).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    Hi, Sorry for the delays, as you see https://github.com/apache/spark/pull/16624 , it's still unknown issue for the existing `SET -v`.
    
    In fact, that is orthogonal to this PR. If we removes the following, this PR will pass. I'll try to fix that TODAY again at there. BTW, if you don't mind, I will remove that test cases for now.
    
    ```
     +  test("SET -v test") {
     +    sql("SET -v").map(_.getString(0)).collect()
     +  }
     +
     +  test("`SET -v` commands should return a list sorted by key") {
     +    val result = sql("SET -v").map(_.getString(0)).collect()
     +    assert(result === result.sorted)
     +  }
    ```
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Just wondering what is the reason why you did not hit this in your local environment? Do you know which conf caused this?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71816 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71816/testReport)** for PR 16579 at commit [`387ab59`](https://github.com/apache/spark/commit/387ab590b8af301433e888e2d7731213e4e254a5).
     * This patch passes all 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Thank you, @gatorsmile , @srowen , and @viirya .


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [WIP][SPARK-19218][SQL] SET command should show a result...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71381 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71381/testReport)** for PR 16579 at commit [`337d02d`](https://github.com/apache/spark/commit/337d02d3fcdec696eb9e89bcad8094d5b34c1f59).
     * 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    For `SET -v` without sorting, please refer #16624 , too.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71816/
    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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    The only failure is irrelevant to this PR.
    ```
    [info] - set spark.sql.warehouse.dir *** FAILED *** (5 minutes, 0 seconds)
    [info]   Timeout of './bin/spark-submit' '--class' 'org.apache.spark.sql.hive.SetWarehouseLocationTest' '--name' 'SetSparkWarehouseLocationTest' '--master' 'local-cluster[2,1,1024]' '--conf' 
    ```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] SET command should show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r96474389
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,7 +79,7 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq.sortBy(_.getString(0))
    --- End diff --
    
    Have you found the reason?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97249317
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    However, IMO, it's needed if there occurs some regression for this case in the future.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71816 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71816/testReport)** for PR 16579 at commit [`387ab59`](https://github.com/apache/spark/commit/387ab590b8af301433e888e2d7731213e4e254a5).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71822/
    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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    Could you just add a test case?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71363/
    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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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

    https://github.com/apache/spark/pull/16579#discussion_r97249538
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    but you don't catch anything actually? so if any regression in the future, is it different with a try or not? you still see an exception.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] SET command should show a result sort...

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

    https://github.com/apache/spark/pull/16579
  
    **[Test build #71342 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71342/testReport)** for PR 16579 at commit [`052a2f4`](https://github.com/apache/spark/commit/052a2f489c8e7e4889a64a673fdce5df2422f2cb).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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 issue #16579: [SPARK-19218][SQL] Fix SET command to show a result corr...

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

    https://github.com/apache/spark/pull/16579
  
    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


[GitHub] spark pull request #16579: [WIP][SPARK-19218][SQL] SET command should show a...

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/16579#discussion_r96509362
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/SetCommand.scala ---
    @@ -79,7 +79,7 @@ case class SetCommand(kv: Option[(String, Option[String])]) extends RunnableComm
         // Queries all key-value pairs that are set in the SQLConf of the sparkSession.
         case None =>
           val runFunc = (sparkSession: SparkSession) => {
    -        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq
    +        sparkSession.conf.getAll.map { case (k, v) => Row(k, v) }.toSeq.sortBy(_.getString(0))
    --- End diff --
    
    Oh, @gatorsmile . I missed your comment. The return value from `sql("set -v")` seems not to be safe. I think there may be some synchronization issue here. I'll create a separate PR for 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 #16579: [SPARK-19218][SQL] Fix SET command to show a resu...

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/16579#discussion_r97249959
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala ---
    @@ -982,6 +982,33 @@ class SQLQuerySuite extends QueryTest with SharedSQLContext {
         spark.sessionState.conf.clear()
       }
     
    +  test("SPARK-19218 SET command should show a result in a sorted order") {
    +    val overrideConfs = sql("SET").collect()
    +    sql(s"SET test.key3=1")
    +    sql(s"SET test.key2=2")
    +    sql(s"SET test.key1=3")
    +    val result = sql("SET").collect()
    +    assert(result ===
    +      (overrideConfs ++ Seq(
    +        Row("test.key1", "3"),
    +        Row("test.key2", "2"),
    +        Row("test.key3", "1"))).sortBy(_.getString(0))
    +    )
    +    spark.sessionState.conf.clear()
    +  }
    +
    +  test("SPARK-19218 `SET -v` should not fail with null value configuration") {
    +    import SQLConf._
    +    val confEntry = SQLConfigBuilder("spark.test").doc("doc").stringConf.createWithDefault(null)
    +
    +    try {
    +      val result = sql("SET -v").collect()
    +      assert(result === result.sortBy(_.getString(0)))
    --- End diff --
    
    The whole Jenkins test does not fail. You can see the test report in the PR description. Here.
    
    https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71539/testReport/


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If 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