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

[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

GitHub user dilipbiswal opened a pull request:

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

    [SPARK-12102][SQL] Cast a non-nullable struct field to a nullable field during analysis

    Compare both left and right side of the case expression ignoring nullablity when checking for type equality.

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

    $ git pull https://github.com/dilipbiswal/spark spark-12102

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

    https://github.com/apache/spark/pull/10156.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 #10156
    
----
commit 74a8983e71ec8ed72b8a071c9eebbf4598b6269e
Author: Dilip Biswal <db...@us.ibm.com>
Date:   2015-12-05T06:01:34Z

    SPARK-12102 Cast a non-nullable struct field to a nullable field during analysis

----


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r46774393
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala ---
    @@ -91,7 +91,7 @@ trait CaseWhenLike extends Expression {
     
       // both then and else expressions should be considered.
       def valueTypes: Seq[DataType] = (thenList ++ elseValue).map(_.dataType)
    -  def valueTypesEqual: Boolean = valueTypes.distinct.size == 1
    +  def valueTypesEqual: Boolean = valueTypes.map(_.asNullable).distinct.size == 1
    --- End diff --
    
    I feel it is not the right fix. We cannot just cast them all to nullable. I feel the best fix should be when we try to find the common type, we understand non-nullable field can be cast to nullable field.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162321822
  
    **[Test build #47235 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47235/consoleFull)** for PR 10156 at commit [`d117339`](https://github.com/apache/spark/commit/d11733946c5dc4697ec45302ea7e1e5a28ea78aa).
     * 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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166267121
  
    Merged build finished. Test FAILed.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166589255
  
    **[Test build #48183 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48183/consoleFull)** for PR 10156 at commit [`3b715f0`](https://github.com/apache/spark/commit/3b715f0f2ffea400f587a70a9a8a782e3bf14730).
     * 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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r48056874
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala ---
    @@ -91,7 +91,7 @@ trait CaseWhenLike extends Expression {
     
       // both then and else expressions should be considered.
       def valueTypes: Seq[DataType] = (thenList ++ elseValue).map(_.dataType)
    -  def valueTypesEqual: Boolean = valueTypes.distinct.size == 1
    +  def valueTypesEqual: Boolean = valueTypes.map(_.asNullable).distinct.size == 1
    --- End diff --
    
    how about:
    ```
    valueTypes.sliding(2, 1).forall {
      case (dt1, dt2) => DataType.equalsIgnoreNullability(dt1, dt2)
    }
    ```
    
    `DataType.equalsIgnoreNullability` is transitive so this should work.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166551776
  
    **[Test build #48171 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48171/consoleFull)** for PR 10156 at commit [`3b715f0`](https://github.com/apache/spark/commit/3b715f0f2ffea400f587a70a9a8a782e3bf14730).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r48056036
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala ---
    @@ -91,7 +91,7 @@ trait CaseWhenLike extends Expression {
     
       // both then and else expressions should be considered.
       def valueTypes: Seq[DataType] = (thenList ++ elseValue).map(_.dataType)
    -  def valueTypesEqual: Boolean = valueTypes.distinct.size == 1
    +  def valueTypesEqual: Boolean = valueTypes.map(_.asNullable).distinct.size == 1
    --- End diff --
    
    `StructType.asNullable` only handles simple struct, for nested struct, the nullability of inner struct will not change.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r46766446
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala ---
    @@ -1143,4 +1143,10 @@ class DataFrameSuite extends QueryTest with SharedSQLContext {
         val primitiveUDF = udf((i: Int) => i * 2)
         checkAnswer(df.select(primitiveUDF($"age")), Row(44) :: Row(null) :: Nil)
       }
    +
    +  test("SPARK-12102: Ignore nullablity when comparing two sides of case") {
    +    val df = sqlContext.sql(
    --- End diff --
    
    can you put the test under `catalyst`? like in `AnalysisSuite`


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r48056391
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala ---
    @@ -91,7 +91,7 @@ trait CaseWhenLike extends Expression {
     
       // both then and else expressions should be considered.
       def valueTypes: Seq[DataType] = (thenList ++ elseValue).map(_.dataType)
    -  def valueTypesEqual: Boolean = valueTypes.distinct.size == 1
    +  def valueTypesEqual: Boolean = valueTypes.map(_.asNullable).distinct.size == 1
    --- End diff --
    
    I think we can just write down the corrected logic here instead of using `distinct`, i.e. handle array, map and struct comparison ourselves.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162167544
  
    Merged build finished. Test FAILed.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-165337440
  
    @yhuai Hi Yin, was waiting for your feed back before i changed the code for this defect. Do you see a case why this would be a problem if we just ignored nullability to see if types involved in case expression are equal ? 
    
    Also, i think we can do better to handle the struct types in terms of type promotion. If we have 
    left side struct(int) and right side struct(long) , then we should be able to pick the wider type, right ? 
    I am working on coding this up and was thinking of addressing this in another PR.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166017714
  
    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: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-166556355
  
    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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162313874
  
    **[Test build #47235 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47235/consoleFull)** for PR 10156 at commit [`d117339`](https://github.com/apache/spark/commit/d11733946c5dc4697ec45302ea7e1e5a28ea78aa).


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-166219070
  
    @cloud-fan Hi Wenchen, have implemented your suggestion. Please take a look when you get a chance. Thanks !!


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162175288
  
    **[Test build #47221 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47221/consoleFull)** for PR 10156 at commit [`74a8983`](https://github.com/apache/spark/commit/74a8983e71ec8ed72b8a071c9eebbf4598b6269e).
     * 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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166736063
  
    **[Test build #48211 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48211/consoleFull)** for PR 10156 at commit [`581da74`](https://github.com/apache/spark/commit/581da742875d2a6a864d2d45259d42f186ef6d4e).
     * 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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166259466
  
    Merged build finished. Test FAILed.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166570688
  
    **[Test build #48183 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48183/consoleFull)** for PR 10156 at commit [`3b715f0`](https://github.com/apache/spark/commit/3b715f0f2ffea400f587a70a9a8a782e3bf14730).


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162175328
  
    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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166551820
  
    Merged build finished. Test FAILed.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-166760987
  
    Thanks! I have merged it 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: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-165339617
  
    @yhuai Hi Yin, forgot to mention.. i tried to handle just the nulllability difference in findWiderTypeForTwo() like following ..
    
       case (a: StructType, b: StructType) =>
          if (DataType.equalsIgnoreNullability(a, b)) {
            Some(a.asNullable)
          }
          else {
            None
          }
    
    This also works... but feel the original change is simpler unless of course there is any serious issue with it :-)


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-166403008
  
    @cloud-fan thanks.. rebasing to newer code helps :-) 


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-162167875
  
    @cloud-fan when you get a chance , can you please help start a retest ? Its the same intermittent failure about csvreader.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r48292189
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala ---
    @@ -91,7 +91,9 @@ trait CaseWhenLike extends Expression {
     
       // both then and else expressions should be considered.
       def valueTypes: Seq[DataType] = (thenList ++ elseValue).map(_.dataType)
    -  def valueTypesEqual: Boolean = valueTypes.distinct.size == 1
    +  def valueTypesEqual: Boolean = valueTypes.size <= 1 || valueTypes.sliding(2, 1).forall {
    +    case Seq(dt1, dt2) => DataType.equalsIgnoreNullability(dt1, dt2)
    --- End diff --
    
    @yhuai Hi Yin, Thanks !! I have changed it to use sameType. 


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166589361
  
    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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162157012
  
    Can one of the admins verify this patch?


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166017682
  
    **[Test build #48054 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48054/consoleFull)** for PR 10156 at commit [`65de259`](https://github.com/apache/spark/commit/65de25955d609852b7088f754bdece47f47cc470).
     * 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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r48281015
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala ---
    @@ -91,7 +91,9 @@ trait CaseWhenLike extends Expression {
     
       // both then and else expressions should be considered.
       def valueTypes: Seq[DataType] = (thenList ++ elseValue).map(_.dataType)
    -  def valueTypesEqual: Boolean = valueTypes.distinct.size == 1
    +  def valueTypesEqual: Boolean = valueTypes.size <= 1 || valueTypes.sliding(2, 1).forall {
    +    case Seq(dt1, dt2) => DataType.equalsIgnoreNullability(dt1, dt2)
    --- End diff --
    
    Can you use `dt1.sameType(dt2)`? I think we do not need to expose `equalsIgnoreNullability`.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166736425
  
    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: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-166619913
  
    LGTM cc @yhuai 


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r48128695
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala ---
    @@ -274,4 +274,12 @@ class AnalysisSuite extends AnalysisTest {
         assert(lits(1) >= min && lits(1) <= max)
         assert(lits(0) == lits(1))
       }
    +
    +  test("SPARK-12102: Ignore nullablity when comparing two sides of case") {
    +    val caseBranches = Seq((Literal(1) > Literal(0)),
    +      CreateStruct(Seq(Cast(Floor(Literal(10)), IntegerType))),
    +      CreateStruct(Seq(Literal(10))))
    +    val plan = OneRowRelation.select(Alias(CaseWhen(caseBranches), "val")())
    +    assertAnalysisSuccess(plan)
    --- End diff --
    
    ah, you may need to update this branch


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166010389
  
    **[Test build #48054 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48054/consoleFull)** for PR 10156 at commit [`65de259`](https://github.com/apache/spark/commit/65de25955d609852b7088f754bdece47f47cc470).


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-162349876
  
    @yhuai Hi Yin, Thanks for your comments. In this fix, we are only changing the nullability of the value for comparision purpose to see if the left and right types are the same (ignoring the nullablity). To my understanding we are not changing the types for processing but only changing it for comparision purpose. The finding of the common type logic is already in place in CaseWhenCoercion in HiveTypeCoercion when the value types are not equal ? Please let me know what you think ..


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-166568432
  
    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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162162184
  
    **[Test build #47218 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47218/consoleFull)** for PR 10156 at commit [`74a8983`](https://github.com/apache/spark/commit/74a8983e71ec8ed72b8a071c9eebbf4598b6269e).


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166259463
  
    **[Test build #48106 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48106/consoleFull)** for PR 10156 at commit [`a87cc86`](https://github.com/apache/spark/commit/a87cc86cff0454395e77388b6eeba6cd954ab48d).
     * This patch **fails Scala style tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r48123275
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala ---
    @@ -274,4 +274,12 @@ class AnalysisSuite extends AnalysisTest {
         assert(lits(1) >= min && lits(1) <= max)
         assert(lits(0) == lits(1))
       }
    +
    +  test("SPARK-12102: Ignore nullablity when comparing two sides of case") {
    +    val caseBranches = Seq((Literal(1) > Literal(0)),
    +      CreateStruct(Seq(Cast(Floor(Literal(10)), IntegerType))),
    +      CreateStruct(Seq(Literal(10))))
    +    val plan = OneRowRelation.select(Alias(CaseWhen(caseBranches), "val")())
    +    assertAnalysisSuccess(plan)
    --- End diff --
    
    we can simplify this test to:
    ```
    val relation = LocalRelation('a.struct('x.int), 'b.struct('x.int.withNullability(false)))
    val plan = relation.select(CaseWhen(Seq(Literal(true), 'a, 'b)).as("val"))
    assertAnalysisSuccess(plan)
    ```


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166259212
  
    **[Test build #48106 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48106/consoleFull)** for PR 10156 at commit [`a87cc86`](https://github.com/apache/spark/commit/a87cc86cff0454395e77388b6eeba6cd954ab48d).


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r48128315
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/analysis/AnalysisSuite.scala ---
    @@ -274,4 +274,12 @@ class AnalysisSuite extends AnalysisTest {
         assert(lits(1) >= min && lits(1) <= max)
         assert(lits(0) == lits(1))
       }
    +
    +  test("SPARK-12102: Ignore nullablity when comparing two sides of case") {
    +    val caseBranches = Seq((Literal(1) > Literal(0)),
    +      CreateStruct(Seq(Cast(Floor(Literal(10)), IntegerType))),
    +      CreateStruct(Seq(Literal(10))))
    +    val plan = OneRowRelation.select(Alias(CaseWhen(caseBranches), "val")())
    +    assertAnalysisSuccess(plan)
    --- End diff --
    
    @cloud-fan Thank you. I was unable to compile the above snippet test. I modified it slightly like following.
    
    val relation = LocalRelation('a.struct(StructField("x", IntegerType, true))
          ,'b.struct(StructField("x", IntegerType, false)))
    
    Does it look ok ? Or i am missing some imports ?


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162321854
  
    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: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-162161856
  
    ok to 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 pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-162168663
  
    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: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r46766481
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/DataFrameSuite.scala ---
    @@ -1143,4 +1143,10 @@ class DataFrameSuite extends QueryTest with SharedSQLContext {
         val primitiveUDF = udf((i: Int) => i * 2)
         checkAnswer(df.select(primitiveUDF($"age")), Row(44) :: Row(null) :: Nil)
       }
    +
    +  test("SPARK-12102: Ignore nullablity when comparing two sides of case") {
    +    val df = sqlContext.sql(
    --- End diff --
    
    we should put tests in `catalyst` if possible, as they don't need to launch a spark job and can run faster.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166718540
  
    **[Test build #48211 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48211/consoleFull)** for PR 10156 at commit [`581da74`](https://github.com/apache/spark/commit/581da742875d2a6a864d2d45259d42f186ef6d4e).


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-166543060
  
    **[Test build #48171 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/48171/consoleFull)** for PR 10156 at commit [`3b715f0`](https://github.com/apache/spark/commit/3b715f0f2ffea400f587a70a9a8a782e3bf14730).


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

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


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-162280722
  
    @cloud-fan Thanks.. i have moved the test to analysis.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-166541510
  
    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: [SPARK-12102][SQL] Cast a non-nullable struct ...

Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the pull request:

    https://github.com/apache/spark/pull/10156#issuecomment-166541234
  
    @cloud-fan Hi Wehchen, can you please help to trigger a 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 pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162167532
  
    **[Test build #47218 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47218/consoleFull)** for PR 10156 at commit [`74a8983`](https://github.com/apache/spark/commit/74a8983e71ec8ed72b8a071c9eebbf4598b6269e).
     * This patch **fails Spark unit tests**.
     * This patch merges cleanly.
     * This patch adds no public classes.


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

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


[GitHub] spark pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#discussion_r48082109
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/conditionalExpressions.scala ---
    @@ -91,7 +91,7 @@ trait CaseWhenLike extends Expression {
     
       // both then and else expressions should be considered.
       def valueTypes: Seq[DataType] = (thenList ++ elseValue).map(_.dataType)
    -  def valueTypesEqual: Boolean = valueTypes.distinct.size == 1
    +  def valueTypesEqual: Boolean = valueTypes.map(_.asNullable).distinct.size == 1
    --- End diff --
    
    @cloud-fan Thanks Wenchen. I am looking into this 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 pull request: [SPARK-12102][SQL] Cast a non-nullable struct ...

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

    https://github.com/apache/spark/pull/10156#issuecomment-162168831
  
    **[Test build #47221 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/47221/consoleFull)** for PR 10156 at commit [`74a8983`](https://github.com/apache/spark/commit/74a8983e71ec8ed72b8a071c9eebbf4598b6269e).


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