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

[GitHub] spark pull request #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

GitHub user eyalfa opened a pull request:

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

    [Spark-16791] [SQL] cast struct with timestamp field fails

    ## What changes were proposed in this pull request?
    a failing test case + fix to SPARK-16791 (https://issues.apache.org/jira/browse/SPARK-16791)
    
    
    
    ## How was this patch tested?
    added a failing test case to CastSuit, then fixed the Cast code and rerun the entire CastSuit
    


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

    $ git pull https://github.com/eyalfa/spark SPARK-16791_cast_struct_with_timestamp_field_fails

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

    https://github.com/apache/spark/pull/14400.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 #14400
    
----
commit 456de1700549172b2b8d731874cd0f5a5579289b
Author: eyal farago <eyal farago>
Date:   2016-07-29T07:55:28Z

    SPARK-16791_cast_struct_with_timestamp_field_fails: added a failing test case.

commit f13113d817fb8508959367501afe2a5b26be7a53
Author: eyal farago <eyal farago>
Date:   2016-07-29T07:56:43Z

    SPARK-16791_cast_struct_with_timestamp_field_fails: trivial fix.

----


---
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 #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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

    https://github.com/apache/spark/pull/14400#discussion_r72764975
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala ---
    @@ -727,6 +727,19 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
         }
       }
     
    +  test("cast struct with a timestamp field") {
    +    val origSchema = StructType(
    +      Seq(StructField( "tsField", TimestampType, false ))
    +    )
    +    val trgSchema = StructType(
    +      // nine out of ten times I'm casting a struct, it's to normalize its fields nullability
    +      Seq(StructField( "tsField", TimestampType, true ))
    +    )
    +    val inp = Literal.create(InternalRow(0L), origSchema)
    --- End diff --
    
    Should we use the correct type value, i.e. `Timestamp` value here?
    `0L` violates the schema type.


---
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 #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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/14400#discussion_r72880474
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala ---
    @@ -727,6 +727,19 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
         }
       }
     
    +  test("cast struct with a timestamp field") {
    +    val originalSchema = StructType(
    --- End diff --
    
    we usually use `new StructType().add("name", TimestampType, nullable = false)`


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    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 #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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

    https://github.com/apache/spark/pull/14400#discussion_r72764858
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala ---
    @@ -727,6 +727,19 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
         }
       }
     
    +  test("cast struct with a timestamp field") {
    +    val origSchema = StructType(
    +      Seq(StructField( "tsField", TimestampType, false ))
    --- End diff --
    
    nit: remove extra whitespaces and use `nullable = false` for 3rd argument of `StructField`.


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    https://github.com/apache/spark/pull/14400#discussion_r72897142


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    Good catch!
    I have some comments, please check them.
    /cc @marmbrus @cloud-fan 


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    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 #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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

    https://github.com/apache/spark/pull/14400#discussion_r72764866
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala ---
    @@ -727,6 +727,19 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
         }
       }
     
    +  test("cast struct with a timestamp field") {
    +    val origSchema = StructType(
    +      Seq(StructField( "tsField", TimestampType, false ))
    +    )
    +    val trgSchema = StructType(
    +      // nine out of ten times I'm casting a struct, it's to normalize its fields nullability
    +      Seq(StructField( "tsField", TimestampType, true ))
    --- End diff --
    
    nit: remove extra whitespaces and use `nullable = true` for 3rd argument of `StructField`.


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    **[Test build #63076 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63076/consoleFull)** for PR 14400 at commit [`e0549a9`](https://github.com/apache/spark/commit/e0549a998f3d75eeedf4334e9d61ad57dbe52efe).


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    **[Test build #63052 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63052/consoleFull)** for PR 14400 at commit [`6e7f69f`](https://github.com/apache/spark/commit/6e7f69f11e02632837e97c428e39ae3de0926e7b).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    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 #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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

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


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    **[Test build #63052 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/63052/consoleFull)** for PR 14400 at commit [`6e7f69f`](https://github.com/apache/spark/commit/6e7f69f11e02632837e97c428e39ae3de0926e7b).


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    thanks, merging to master and 2.0!


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

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


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    @ueshin , you seem to be the original committer of this code. can you please have a look?


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

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


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    @cloud-fan, just for notice for the next patches: is there a way to run mvn in a mode that fails on style issues? it could save a lot of round-trips if it existed.


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

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


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    Good catch!
    
    LGTM except the style comment given by @ueshin . And please also use full word for variable name, e.g. `originalSchema`, `targetSchema`


---
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 #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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/14400#discussion_r72897142
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala ---
    @@ -727,6 +727,16 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
         }
       }
     
    +  test("cast struct with a timestamp field") {
    +    val originalSchema = new StructType().add("tsField", TimestampType, nullable = false)
    +    // nine out of ten times I'm casting a struct, it's to normalize its fields nullability
    +    val targetSchema = new StructType().add("tsField", TimestampType, nullable = true)
    +
    +    val inp = Literal.create(InternalRow(0L), originalSchema)
    +    val expected = InternalRow(0L)
    +    checkEvaluation(cast(inp, targetSchema), expected )
    --- End diff --
    
    and remove the extra space here too


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

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


[GitHub] spark pull request #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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/14400#discussion_r72889243
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala ---
    @@ -727,6 +727,16 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
         }
       }
     
    +  test("cast struct with a timestamp field") {
    +    val originalSchema = new StructType().add( "tsField", TimestampType, nullable = false )
    --- End diff --
    
    please remove the space in `add( xxx )`


---
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 #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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

    https://github.com/apache/spark/pull/14400#discussion_r72764850
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/Cast.scala ---
    @@ -416,7 +416,7 @@ case class Cast(child: Expression, dataType: DataType) extends UnaryExpression w
       }
     
       private[this] def cast(from: DataType, to: DataType): Any => Any = to match {
    -    case dt if dt == child.dataType => identity[Any]
    +    case dt if dt ==  from => identity[Any]
    --- End diff --
    
    nit: remove extra whitespace before `from`.


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    @cloud-fan, any update on this?


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    we have an individual style check script: `./dev/scalastyle`


---
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 #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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

    https://github.com/apache/spark/pull/14400#discussion_r72764872
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala ---
    @@ -727,6 +727,19 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
         }
       }
     
    +  test("cast struct with a timestamp field") {
    +    val origSchema = StructType(
    +      Seq(StructField( "tsField", TimestampType, false ))
    +    )
    +    val trgSchema = StructType(
    +      // nine out of ten times I'm casting a struct, it's to normalize its fields nullability
    +      Seq(StructField( "tsField", TimestampType, true ))
    +    )
    +    val inp = Literal.create(InternalRow(0L), origSchema)
    +    val expected = InternalRow(0L)
    +    checkEvaluation( Cast(inp, trgSchema), expected)
    --- End diff --
    
    nit: remove extra whitespace before `Cast` and you can use `cast` instead of `Cast` here.


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

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


[GitHub] spark pull request #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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

    https://github.com/apache/spark/pull/14400#discussion_r72769633
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala ---
    @@ -727,6 +727,19 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
         }
       }
     
    +  test("cast struct with a timestamp field") {
    +    val origSchema = StructType(
    +      Seq(StructField( "tsField", TimestampType, false ))
    +    )
    +    val trgSchema = StructType(
    +      // nine out of ten times I'm casting a struct, it's to normalize its fields nullability
    +      Seq(StructField( "tsField", TimestampType, true ))
    +    )
    +    val inp = Literal.create(InternalRow(0L), origSchema)
    --- End diff --
    
    This is an internal row,timestamps are represented as longs.
    We can probably use a more meaningful date literal


---
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 #14400: [Spark-16791] [SQL] cast struct with timestamp fi...

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

    https://github.com/apache/spark/pull/14400#discussion_r72770823
  
    --- Diff: sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CastSuite.scala ---
    @@ -727,6 +727,19 @@ class CastSuite extends SparkFunSuite with ExpressionEvalHelper {
         }
       }
     
    +  test("cast struct with a timestamp field") {
    +    val origSchema = StructType(
    +      Seq(StructField( "tsField", TimestampType, false ))
    +    )
    +    val trgSchema = StructType(
    +      // nine out of ten times I'm casting a struct, it's to normalize its fields nullability
    +      Seq(StructField( "tsField", TimestampType, true ))
    +    )
    +    val inp = Literal.create(InternalRow(0L), origSchema)
    --- End diff --
    
    Ah, I see.


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

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


[GitHub] spark issue #14400: [Spark-16791] [SQL] cast struct with timestamp field fai...

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

    https://github.com/apache/spark/pull/14400
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/63052/
    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