Skip to content

Conversation

@cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

For input object of non-flat type, we can't encode it to row if it's null, as Spark SQL doesn't allow row to be null, only its columns can be null.

This PR explicitly add this constraint and throw exception if users break it.

How was this patch tested?

several new tests

@cloud-fan
Copy link
Contributor Author

cc @marmbrus @liancheng @clockfly

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried to also disallow null input object for flat type, but failed as some other tests already depend on this feature. e.g. ParquetIOSuite.null and non-null strings

@SparkQA
Copy link

SparkQA commented Jun 2, 2016

Test build #59854 has finished for PR 13469 at commit f546328.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Jun 2, 2016

Test build #59860 has finished for PR 13469 at commit de1eff6.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

inputObject
} else {
// For input object of non-flat type, we can't encode it to row if it's null, as Spark SQL
// doesn't allow row to be null, only its columns can be null.
Copy link
Contributor

Choose a reason for hiding this comment

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

... doesn't allow top-level row to be null, ...

@liancheng
Copy link
Contributor

LGTM except for minor comments.

@liancheng
Copy link
Contributor

Let's wait for #13269 first.

@liancheng
Copy link
Contributor

LGTM pending Jenkins.

@SparkQA
Copy link

SparkQA commented Jun 3, 2016

Test build #59941 has finished for PR 13469 at commit 6b2641f.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@liancheng
Copy link
Contributor

Merging to master and branch-2.0.

asfgit pushed a commit that referenced this pull request Jun 3, 2016
…r clear

## What changes were proposed in this pull request?

For input object of non-flat type, we can't encode it to row if it's null, as Spark SQL doesn't allow row to be null, only its columns can be null.

This PR explicitly add this constraint and throw exception if users break it.

## How was this patch tested?

several new tests

Author: Wenchen Fan <[email protected]>

Closes #13469 from cloud-fan/null-object.

(cherry picked from commit 11c83f8)
Signed-off-by: Cheng Lian <[email protected]>
@asfgit asfgit closed this in 11c83f8 Jun 3, 2016
asfgit pushed a commit that referenced this pull request Nov 30, 2016
## What changes were proposed in this pull request?

For input object of non-flat type, we can't encode it to row if it's null, as Spark SQL doesn't allow the entire row to be null, only its columns can be null. That's the reason we forbid users to use top level null objects in #13469

However, if users wrap non-flat type with `Option`, then we may still encoder top level null object to row, which is not allowed.

This PR fixes this case, and suggests users to wrap their type with `Tuple1` if they do wanna top level null objects.

## How was this patch tested?

new test

Author: Wenchen Fan <[email protected]>

Closes #15979 from cloud-fan/option.
asfgit pushed a commit that referenced this pull request Nov 30, 2016
## What changes were proposed in this pull request?

For input object of non-flat type, we can't encode it to row if it's null, as Spark SQL doesn't allow the entire row to be null, only its columns can be null. That's the reason we forbid users to use top level null objects in #13469

However, if users wrap non-flat type with `Option`, then we may still encoder top level null object to row, which is not allowed.

This PR fixes this case, and suggests users to wrap their type with `Tuple1` if they do wanna top level null objects.

## How was this patch tested?

new test

Author: Wenchen Fan <[email protected]>

Closes #15979 from cloud-fan/option.

(cherry picked from commit f135b70)
Signed-off-by: Cheng Lian <[email protected]>
robert3005 pushed a commit to palantir/spark that referenced this pull request Dec 2, 2016
## What changes were proposed in this pull request?

For input object of non-flat type, we can't encode it to row if it's null, as Spark SQL doesn't allow the entire row to be null, only its columns can be null. That's the reason we forbid users to use top level null objects in apache#13469

However, if users wrap non-flat type with `Option`, then we may still encoder top level null object to row, which is not allowed.

This PR fixes this case, and suggests users to wrap their type with `Tuple1` if they do wanna top level null objects.

## How was this patch tested?

new test

Author: Wenchen Fan <[email protected]>

Closes apache#15979 from cloud-fan/option.
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
## What changes were proposed in this pull request?

For input object of non-flat type, we can't encode it to row if it's null, as Spark SQL doesn't allow the entire row to be null, only its columns can be null. That's the reason we forbid users to use top level null objects in apache#13469

However, if users wrap non-flat type with `Option`, then we may still encoder top level null object to row, which is not allowed.

This PR fixes this case, and suggests users to wrap their type with `Tuple1` if they do wanna top level null objects.

## How was this patch tested?

new test

Author: Wenchen Fan <[email protected]>

Closes apache#15979 from cloud-fan/option.
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.

3 participants