Skip to content

Conversation

@ueshin
Copy link
Member

@ueshin ueshin commented Apr 24, 2014

Cast.nullable should be true when cast from StringType to NumericType or TimestampType.
Because if StringType expression has an illegal number string or illegal timestamp string, the casted value becomes null.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14438/

@marmbrus
Copy link
Contributor

Hey, thanks for this fix too! I'm a little torn about adding the nullability, foldability and datatype checks to all the evaluation tests. At the very least, we need to use named parameters with Boolean arguments to keep things readable. However, I think a better option might be to separate these checks out into their own unit tests. Doing so might decease the amount of redundancy and make it more obvious what cases are actually being tested. Mostly I want to keep a very concise way to write expression evaluation unit tests so that it is easy to keep adding them. What do you think?

@ueshin
Copy link
Member Author

ueshin commented Apr 25, 2014

Hi, thank you for your reply.

I thought that all expressions should be checked in the same way but now I agree that the tests are not readable and concise.
I'll revert them and add some specific tests for this issue.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build triggered.

@AmplabJenkins
Copy link

Merged build started.

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14473/

@AmplabJenkins
Copy link

Merged build finished. All automated tests passed.

@AmplabJenkins
Copy link

All automated tests passed.
Refer to this link for build results: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/14474/

@marmbrus
Copy link
Contributor

This LGTM. Thanks!

@rxin or @pwendell can you merge in master and 1.0? Thx!

@rxin
Copy link
Contributor

rxin commented Apr 26, 2014

Thanks. Merged.

asfgit pushed a commit that referenced this pull request Apr 26, 2014
…ericType/TimestampType.

`Cast.nullable` should be `true` when cast from `StringType` to `NumericType` or `TimestampType`.
Because if `StringType` expression has an illegal number string or illegal timestamp string, the casted value becomes `null`.

Author: Takuya UESHIN <[email protected]>

Closes #532 from ueshin/issues/SPARK-1608 and squashes the following commits:

065d37c [Takuya UESHIN] Add tests to check nullabilities of cast expressions.
f278ed7 [Takuya UESHIN] Revert test to keep it readable and concise.
9fc9380 [Takuya UESHIN] Fix Cast.nullable when cast from StringType to NumericType/TimestampType.

(cherry picked from commit 8e37ed6)
Signed-off-by: Reynold Xin <[email protected]>
@asfgit asfgit closed this in 8e37ed6 Apr 26, 2014
pdeyhim pushed a commit to pdeyhim/spark-1 that referenced this pull request Jun 25, 2014
…ericType/TimestampType.

`Cast.nullable` should be `true` when cast from `StringType` to `NumericType` or `TimestampType`.
Because if `StringType` expression has an illegal number string or illegal timestamp string, the casted value becomes `null`.

Author: Takuya UESHIN <[email protected]>

Closes apache#532 from ueshin/issues/SPARK-1608 and squashes the following commits:

065d37c [Takuya UESHIN] Add tests to check nullabilities of cast expressions.
f278ed7 [Takuya UESHIN] Revert test to keep it readable and concise.
9fc9380 [Takuya UESHIN] Fix Cast.nullable when cast from StringType to NumericType/TimestampType.
yifeih pushed a commit to yifeih/spark that referenced this pull request May 8, 2019
apache#532)

* [SPARK-25299] Use the shuffle writer plugin for the SortShuffleWriter.

* Remove unused

* Handle empty partitions properly.

* Adjust formatting

* Don't close streams twice.

Because compressed output streams don't like it.

* Clarify comment
squito pushed a commit to squito/spark that referenced this pull request Jul 18, 2019
apache#532)

* [SPARK-25299] Use the shuffle writer plugin for the SortShuffleWriter.

* Remove unused

* Handle empty partitions properly.

* Adjust formatting

* Don't close streams twice.

Because compressed output streams don't like it.

* Clarify comment
turboFei pushed a commit to turboFei/spark that referenced this pull request Nov 6, 2025
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.

4 participants