Skip to content

Comments

[SPARK-28706][SQL] Allow cast null type to any types#25425

Closed
jiangxb1987 wants to merge 2 commits intoapache:masterfrom
jiangxb1987:nullToString
Closed

[SPARK-28706][SQL] Allow cast null type to any types#25425
jiangxb1987 wants to merge 2 commits intoapache:masterfrom
jiangxb1987:nullToString

Conversation

@jiangxb1987
Copy link
Contributor

@jiangxb1987 jiangxb1987 commented Aug 13, 2019

What changes were proposed in this pull request?

#25242 proposed to disallow upcasting complex data types to string type, however, upcasting from null type to any types should still be safe.

How was this patch tested?

Add corresponding case in CastSuite.

@jiangxb1987
Copy link
Contributor Author

case (DateType, TimestampType) => true
case (_: AtomicType, StringType) => true
case (_: CalendarIntervalType, StringType) => true
case (NullType, StringType) => true
Copy link
Member

Choose a reason for hiding this comment

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

So this is a logical partial recover of the previous code case (_, StringType) => true before #25242, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can cast null type to any type, how about case (NullType, _) => true?

Copy link
Member

Choose a reason for hiding this comment

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

+1, @cloud-fan 's comment.

In that case, let's create a new JIRA instead of FOLLOWUP, @jiangxb1987 .

Copy link
Member

Choose a reason for hiding this comment

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

+1, @cloud-fan 's comment.

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

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

+1, LGTM.

@SparkQA
Copy link

SparkQA commented Aug 13, 2019

Test build #109004 has finished for PR 25425 at commit c746d4f.

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

@jiangxb1987 jiangxb1987 changed the title [SPARK-28497][FOLLOWUP][SQL] Allow cast NullType to StringType [SPARK-28706][SQL] Allow cast null type to any types Aug 13, 2019
@SparkQA
Copy link

SparkQA commented Aug 13, 2019

Test build #109015 has finished for PR 25425 at commit fceebda.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jiangxb1987
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Aug 13, 2019

Test build #109026 has finished for PR 25425 at commit fceebda.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@cloud-fan cloud-fan closed this in 3249c7a Aug 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants