Skip to content

Conversation

@cloud-fan
Copy link
Contributor

nullability should only be considered as an optimization rather than part of the type system, so instead of failing analysis for mismatch nullability, we should pass analysis and add runtime null check.

@cloud-fan
Copy link
Contributor Author

cc @marmbrus @liancheng

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous tests are very verbose and hard to maintain, every time we change some unrelated analysis behaviours, these tests need to be updated. Actually for these tests, we only care about if analysis passes, runtime execution successes. So I simplified these tests to only ensure we don't fail analysis and execution.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

@SparkQA
Copy link

SparkQA commented Feb 3, 2016

Test build #50611 has finished for PR 11035 at commit aa07e56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])

@liancheng
Copy link
Contributor

retest this please

@SparkQA
Copy link

SparkQA commented Feb 3, 2016

Test build #50617 has finished for PR 11035 at commit aa07e56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])

@cloud-fan
Copy link
Contributor Author

retest this please

@liancheng
Copy link
Contributor

I'm investigating the failed flaky tests, and @davies is marking them as ignored temporarily to bring back the PR builder.

@SparkQA
Copy link

SparkQA commented Feb 3, 2016

Test build #50620 has finished for PR 11035 at commit aa07e56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])

@cloud-fan
Copy link
Contributor Author

retest it please

@liancheng
Copy link
Contributor

LGTM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So this does still silently corrupt values.

scala> Seq(("a", Seq(null, new Integer(1)))).toDS().as[(String, Array[Int])].collect()
res5: Array[(String, Array[Int])] = Array((a,Array(0, 1)))

Since this isn't a regression its probably okay as long as we open another JIRA.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SparkQA
Copy link

SparkQA commented Feb 3, 2016

Test build #2507 has finished for PR 11035 at commit aa07e56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])

@cloud-fan
Copy link
Contributor Author

retest this please

asfgit pushed a commit that referenced this pull request Feb 4, 2016
…ld not fail analysis of encoder

nullability should only be considered as an optimization rather than part of the type system, so instead of failing analysis for mismatch nullability, we should pass analysis and add runtime null check.

backport #11035 to 1.6

Author: Wenchen Fan <[email protected]>

Closes #11042 from cloud-fan/branch-1.6.
@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Feb 6, 2016

Test build #50860 has finished for PR 11035 at commit aa07e56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Feb 6, 2016

Test build #50861 has finished for PR 11035 at commit aa07e56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Feb 6, 2016

Test build #50864 has finished for PR 11035 at commit aa07e56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])

@gatorsmile
Copy link
Member

retest this please

@SparkQA
Copy link

SparkQA commented Feb 6, 2016

Test build #50871 has finished for PR 11035 at commit aa07e56.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Feb 7, 2016

Test build #50886 has finished for PR 11035 at commit aa07e56.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds the following public classes (experimental):
    • case class AssertNotNull(child: Expression, walkedTypePath: Seq[String])

@cloud-fan
Copy link
Contributor Author

cc @marmbrus

@marmbrus
Copy link
Contributor

marmbrus commented Feb 8, 2016

Thanks, merging to master.

@asfgit asfgit closed this in 8e4d15f Feb 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants