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

[GitHub] spark pull request #21321: [SPARK-24268][SQL] Use datatype.simpleString in e...

GitHub user mgaido91 opened a pull request:

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

    [SPARK-24268][SQL] Use datatype.simpleString in error messages

    ## What changes were proposed in this pull request?
    
    SPARK-22893 tried to unify error messages about dataTypes. Unfortunately, still many places were missing the `simpleString` method in other to have the same representation everywhere.
    
    The PR unified the messages using alway the simpleString representation of the dataTypes in the messages.
    
    ## How was this patch tested?
    
    existing/modified UTs


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

    $ git pull https://github.com/mgaido91/spark SPARK-24268

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

    https://github.com/apache/spark/pull/21321.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 #21321
    
----
commit ada7667a9f8872f373bc789a7eb0c84987642314
Author: Marco Gaido <ma...@...>
Date:   2018-05-05T15:19:45Z

    [SPARK-24268][SQL] Use datatype.simpleString in error messages

----


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    For instance, https://github.com/apache/spark/pull/20064 at SPARK-22893, right? Let's fix them too. Yes, since this one is reverted, we should better stick to `catalogString` in all such places, which I believe we all agreed upon here now.
    
    I only haven't left some comments about this so far only because my impression was that we were not yet sure between `simpleString` and `catalogString` in error messages.
    
    Now it looks at least including me three committer prefers `catalogString` in error messages.


---

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


[GitHub] spark pull request #21321: [SPARK-24268][SQL] Use datatype.simpleString in e...

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

    https://github.com/apache/spark/pull/21321#discussion_r198968646
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/FeatureHasher.scala ---
    @@ -208,7 +208,8 @@ class FeatureHasher(@Since("2.3.0") override val uid: String) extends Transforme
           require(dataType.isInstanceOf[NumericType] ||
    --- End diff --
    
    Another option would be to use SchemaUtils `checkColumnTypes` here -- what do you think?


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    **[Test build #92747 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/92747/testReport)** for PR 21321 at commit [`781b64d`](https://github.com/apache/spark/commit/781b64d4944a801cfc3128f8bc3ec12924da5549).


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    That was the thing I was hesitant of. I was kind of confused recently about this because `simpleString` was used in some recent PRs even although I know `catalogString` is preferred. If this fix is "wrong", other PRs using `simpleString` all should be reverted though.
    
    @mgaido91, mind opening a PR with `catalogString` again please?


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    I think the fix is wrong. We should not use simpleString but catalogString, because simpleString will do the truncation. 


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    Thanks for your kind and nice review @holdenk, I addressed the places I missed. Thank you!


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    cc @gatorsmile 


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    @HyukjinKwon The whole PR is doing the wrong things. That is why I reverted it. I do not want the others to follow this PR. For the other PRs whose main objective are not to use `simpleString` to improve the error message, we can submit follow-up PRs to fix them; otherwise, feel free to revert them WDYT?


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    kindly ping @gatorsmile 


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    @mgaido91, mind rebasing this please? Let me just get this in.


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    `catalogString ` is preferred in error messages.


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    @gatorsmile I used `simpleString` because in the previous JIRA which attempted to unify error messages `simpleString` was used, as well as I have always seen that method being used. Anyway, I am fine with `catalogString` too, but then we should change also the current places which are using `simpleString`, do we agree on this solution?


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark pull request #21321: [SPARK-24268][SQL] Use datatype.simpleString in e...

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

    https://github.com/apache/spark/pull/21321#discussion_r198964798
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/FeatureHasher.scala ---
    @@ -208,7 +208,8 @@ class FeatureHasher(@Since("2.3.0") override val uid: String) extends Transforme
           require(dataType.isInstanceOf[NumericType] ||
             dataType.isInstanceOf[StringType] ||
             dataType.isInstanceOf[BooleanType],
    -        s"FeatureHasher requires columns to be of NumericType, BooleanType or StringType. " +
    +        s"FeatureHasher requires columns to be of ${NumericType.simpleString}, " +
    --- End diff --
    
    I think that's my bad, looking through it I saw some raw type names but those are all in the suites which makes sense.


---

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


[GitHub] spark pull request #21321: [SPARK-24268][SQL] Use datatype.simpleString in e...

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

    https://github.com/apache/spark/pull/21321#discussion_r195156323
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/FeatureHasher.scala ---
    @@ -208,7 +208,8 @@ class FeatureHasher(@Since("2.3.0") override val uid: String) extends Transforme
           require(dataType.isInstanceOf[NumericType] ||
             dataType.isInstanceOf[StringType] ||
             dataType.isInstanceOf[BooleanType],
    -        s"FeatureHasher requires columns to be of NumericType, BooleanType or StringType. " +
    +        s"FeatureHasher requires columns to be of ${NumericType.simpleString}, " +
    --- End diff --
    
    In the original PR it doesn't seem like we rewrote the constant types only the dynamic ones (and this PR also doesn't seem to consistently rewrite the constant types referenced). What's the reason/how do you decide which ones you want to rewrite to ref simpleString?


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    Ah, it's fine. This one is already reverted and I don't mind. Not a big deal actually. Glad that we are on the same page about `catalogString` anyway.


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    kindly ping @gatorsmile


---

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


[GitHub] spark pull request #21321: [SPARK-24268][SQL] Use datatype.simpleString in e...

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

    https://github.com/apache/spark/pull/21321#discussion_r199091397
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/FeatureHasher.scala ---
    @@ -208,7 +208,8 @@ class FeatureHasher(@Since("2.3.0") override val uid: String) extends Transforme
           require(dataType.isInstanceOf[NumericType] ||
    --- End diff --
    
    I couldn't do that because `NumericType` is an `AbstractDataType` and not a `DataType` so I couldn't use that method.


---

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


[GitHub] spark pull request #21321: [SPARK-24268][SQL] Use datatype.simpleString in e...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    Merged to master.


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    Let me revert the changes. Please re-submit the fix. 


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    sorry for the late answer @HyukjinKwon. I just rebased it, thanks.


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    `jsonExpressions.scala` also has the same issue?
    https://github.com/apache/spark/blob/b6c50d7820aafab172835633fb0b35899e93146b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/jsonExpressions.scala#L756
    
    Then, you need to update the output results in `SQLQueryTestSuite`.


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    **[Test build #90586 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/90586/testReport)** for PR 21321 at commit [`ada7667`](https://github.com/apache/spark/commit/ada7667a9f8872f373bc789a7eb0c84987642314).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

    https://github.com/apache/spark/pull/21321
  
    thanks @maropu I missed that one. I'll update it shortly, thanks.


---

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


[GitHub] spark issue #21321: [SPARK-24268][SQL] Use datatype.simpleString in error me...

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

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


---

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


[GitHub] spark pull request #21321: [SPARK-24268][SQL] Use datatype.simpleString in e...

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

    https://github.com/apache/spark/pull/21321#discussion_r195344259
  
    --- Diff: mllib/src/main/scala/org/apache/spark/ml/feature/FeatureHasher.scala ---
    @@ -208,7 +208,8 @@ class FeatureHasher(@Since("2.3.0") override val uid: String) extends Transforme
           require(dataType.isInstanceOf[NumericType] ||
             dataType.isInstanceOf[StringType] ||
             dataType.isInstanceOf[BooleanType],
    -        s"FeatureHasher requires columns to be of NumericType, BooleanType or StringType. " +
    +        s"FeatureHasher requires columns to be of ${NumericType.simpleString}, " +
    --- End diff --
    
    I think this PR rewrites always constant type referenced. I am not sure why you are saying it is not. If I missed some places, then it was just because I haven't seen them.


---

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