You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by koertkuipers <gi...@git.apache.org> on 2016/03/26 21:53:48 UTC

[GitHub] spark pull request: SPARK-14139 Dataset loses nullability in opera...

GitHub user koertkuipers opened a pull request:

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

    SPARK-14139 Dataset loses nullability in operations with RowEncoder

    ## What changes were proposed in this pull request?
    
    RowEncoder now respects nullability for struct fields when creating extractor expressions in the extractorsFor method.
    
    Note that to get the correct value for nullable for the returned expression i chose to drop the If statement checking for nulls if the field has nullable=false. If this is undesired because we should defensively be checking for nulls anyhow with the If statement then that can be achieved as well, by modifying the If class, however to me that solution seems less clear/elegant.
    
    ## How was this patch tested?
    
    Added new unit test in DataFrameSuite for the bug described in the jira issue.


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

    $ git pull https://github.com/tresata/spark feat-rowencoder-nullable

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

    https://github.com/apache/spark/pull/11980.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 #11980
    
----
commit 847d7c7cdfe6626ea1f73656f9eaf868d641ae1c
Author: Koert Kuipers <ko...@tresata.com>
Date:   2016-03-26T20:14:18Z

    change RowEncoder to respect nullable for struct fields when generating extractors

commit b500c8bb1d3d8f1ea1d88a2f761056b946598871
Author: Koert Kuipers <ko...@tresata.com>
Date:   2016-03-26T20:46:54Z

    merge from 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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-203069529
  
    **[Test build #54457 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54457/consoleFull)** for PR 11980 at commit [`50c0005`](https://github.com/apache/spark/commit/50c00054f601d3b59f23493ba4ab46e6d2c1ac03).


---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-201929188
  
    **[Test build #54264 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54264/consoleFull)** for PR 11980 at commit [`b500c8b`](https://github.com/apache/spark/commit/b500c8bb1d3d8f1ea1d88a2f761056b946598871).


---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#discussion_r58319194
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala ---
    @@ -120,17 +120,19 @@ object RowEncoder {
     
         case StructType(fields) =>
           val convertedFields = fields.zipWithIndex.map { case (f, i) =>
    -        val method = if (f.dataType.isInstanceOf[StructType]) {
    -          "getStruct"
    +        val x = extractorsFor(
    +          GetExternalRowField(inputObject, i, externalDataTypeForInput(f.dataType), f.nullable),
    +          f.dataType
    +        )
    +        if (f.nullable) {
    +          If(
    --- End diff --
    
    if we do the null check inside GetExternalRowField then the code for serializerFor also needs to be pushed into it (to be inside the null check), and i can not figure out how to do that...


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

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


[GitHub] spark pull request: SPARK-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-203106835
  
    **[Test build #54457 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54457/consoleFull)** for PR 11980 at commit [`50c0005`](https://github.com/apache/spark/commit/50c00054f601d3b59f23493ba4ab46e6d2c1ac03).
     * 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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-203107235
  
    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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-203070056
  
    @cloud-fan i pushed at attempt at this, but i am having trouble with RowEncoderSuite encode/decode: Product
    this test uses a Product value with a StructType schema, and RowEncoder is supposed to handle this, but i can not figure out how this worked before.


---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-201943850
  
    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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#discussion_r57829829
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala ---
    @@ -120,17 +120,19 @@ object RowEncoder {
     
         case StructType(fields) =>
           val convertedFields = fields.zipWithIndex.map { case (f, i) =>
    -        val method = if (f.dataType.isInstanceOf[StructType]) {
    -          "getStruct"
    +        val x = extractorsFor(
    +          GetExternalRowField(inputObject, i, externalDataTypeForInput(f.dataType), f.nullable),
    +          f.dataType
    +        )
    +        if (f.nullable) {
    +          If(
    --- End diff --
    
    i had that before (basically without the If, since GetExternalRowField already does null checks inside), but the issue is that extractorsFor code path then also runs for nulls , and that causes some weird runtime errors. i will try again.


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

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


[GitHub] spark pull request: SPARK-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-205060396
  
    **[Test build #54809 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54809/consoleFull)** for PR 11980 at commit [`e9a9a30`](https://github.com/apache/spark/commit/e9a9a30e1804785d3534bea78cf2ce588f7fc51b).
     * 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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-202588020
  
    @cloud-fan i tried to do that, but i don't think i am familiar enough with the code gen, because it breaks other unit tests. it seems to me i am messing up with internal vs external types.
    you want me to push my broken code?
    
    



---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-212182404
  
    (This is trivial but might be better if the title follows `[SPARK-XXXXX][SQL]` format as described in https://cwiki.apache.org/confluence/display/SPARK/Contributing+to+Spark.)


---
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-14139 Dataset loses nullability in opera...

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/11980#discussion_r57827339
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects.scala ---
    @@ -680,3 +680,54 @@ case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])
          """
       }
     }
    +
    +case class GetExternalRowField(
    --- End diff --
    
    You can take a look at `GetStructFiled`, which gets field from internal row, and is similar to this one.


---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-201929263
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54264/
    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-14139 Dataset loses nullability in opera...

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

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


---
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-14139 Dataset loses nullability in opera...

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/11980#discussion_r57827201
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/encoders/RowEncoder.scala ---
    @@ -120,17 +120,19 @@ object RowEncoder {
     
         case StructType(fields) =>
           val convertedFields = fields.zipWithIndex.map { case (f, i) =>
    -        val method = if (f.dataType.isInstanceOf[StructType]) {
    -          "getStruct"
    +        val x = extractorsFor(
    +          GetExternalRowField(inputObject, i, externalDataTypeForInput(f.dataType), f.nullable),
    +          f.dataType
    +        )
    +        if (f.nullable) {
    +          If(
    --- End diff --
    
    I think we can do the null check inside `GetExternalRowField`


---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-203107239
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54457/
    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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-205046022
  
    **[Test build #54809 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54809/consoleFull)** for PR 11980 at commit [`e9a9a30`](https://github.com/apache/spark/commit/e9a9a30e1804785d3534bea78cf2ce588f7fc51b).


---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-205060653
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54809/
    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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-201929929
  
    **[Test build #54265 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54265/consoleFull)** for PR 11980 at commit [`3e32b6a`](https://github.com/apache/spark/commit/3e32b6aa4dbc0adcdd892ee838ccaed77a67dc58).


---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-201943853
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/54265/
    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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-201929261
  
    **[Test build #54264 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54264/consoleFull)** for PR 11980 at commit [`b500c8b`](https://github.com/apache/spark/commit/b500c8bb1d3d8f1ea1d88a2f761056b946598871).
     * 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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-201943320
  
    **[Test build #54265 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/54265/consoleFull)** for PR 11980 at commit [`3e32b6a`](https://github.com/apache/spark/commit/3e32b6aa4dbc0adcdd892ee838ccaed77a67dc58).
     * 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-14139 Dataset loses nullability in opera...

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/11980#discussion_r57546554
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects.scala ---
    @@ -109,9 +109,9 @@ case class Invoke(
         targetObject: Expression,
         functionName: String,
         dataType: DataType,
    -    arguments: Seq[Expression] = Nil) extends Expression with NonSQLExpression {
    +    arguments: Seq[Expression] = Nil,
    +    val nullable: Boolean = true) extends Expression with NonSQLExpression {
    --- End diff --
    
    This looks tricky to me, invoking a method on an object should be always nullable, as we have no idea what that method does. To fix the nullability lose issue, how about we create a new expression called `GetExternalRowField`? We can setup corrected nullability there and generate simpler code for this case.


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

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


[GitHub] spark pull request: SPARK-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-201929262
  
    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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#discussion_r57829840
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/objects.scala ---
    @@ -680,3 +680,54 @@ case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])
          """
       }
     }
    +
    +case class GetExternalRowField(
    --- End diff --
    
    ok will do


---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-220493938
  
    @koertkuipers this can be closed now right?



---
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-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-202123640
  
    cc @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 pull request: SPARK-14139 Dataset loses nullability in opera...

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

    https://github.com/apache/spark/pull/11980#issuecomment-205060651
  
    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