[SPARK-24586][SQL] Upcast should not allow casting from string to other types by cloud-fan · Pull Request #21586 · apache/spark

cloud-fan · 2018-06-18T23:24:37Z

What changes were proposed in this pull request?

When turning a Dataset to another Dataset, Spark will up cast the fields in the original Dataset to the type of corresponding fields in the target DataSet.

However, the current upcast behavior is a little weird, we don't allow up casting from string to numeric, but allow non-numeric types as the target, like boolean, date, etc.

As a result, Seq("str").toDS.as[Int] fails, but Seq("str").toDS.as[Boolean] works and throw NPE during execution.

The motivation of the up cast is to prevent things like runtime NPE, it's more reasonable to make up cast stricter.

This PR does 2 things:

rename Cast.canSafeCast to Cast.canUpcast, and support complex typres
remove Cast.mayTruncate and replace it with !Cast.canUpcast

Note that, the up cast change also affects persistent view resolution. But since we don't support changing column types of an existing table, there is no behavior change here.

How was this patch tested?

new tests

cloud-fan · 2018-06-18T23:24:54Z

cc @marmbrus @viirya @gatorsmile

SparkQA · 2018-06-19T01:52:28Z

Test build #92052 has finished for PR 21586 at commit c89d12e.

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

maropu · 2018-06-19T02:26:51Z

retest this please

viirya · 2018-06-19T02:49:14Z

Is the error message of thrown AnalysisException valid under this change? Seems it is not just because truncating?

maropu · 2018-06-19T02:50:11Z

super nit: fromPrecedence != -1 && fromPrecedence < toPrecedence is more easy-to-understand?

viirya · 2018-06-19T03:06:34Z

Seems nullable of StructField can affect the casting decision?

added support for complex types

SparkQA · 2018-06-19T04:27:41Z

Test build #92059 has finished for PR 21586 at commit c89d12e.

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

…is wrapped in Cast ## What changes were proposed in this pull request? As mentioned in #21586 , `Cast.mayTruncate` is not 100% safe, string to boolean is allowed. Since changing `Cast.mayTruncate` also changes the behavior of Dataset, here I propose to add a new `Cast.canSafeCast` for partition pruning. ## How was this patch tested? new test cases Author: Wenchen Fan <wenchen@databricks.com> Closes#21712 from cloud-fan/safeCast.

SparkQA · 2018-08-03T07:05:01Z

Test build #94103 has finished for PR 21586 at commit c89d12e.

This patch fails due to an unknown error code, -9.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2019-04-11T14:00:21Z

Test build #104512 has finished for PR 21586 at commit 2a64d59.

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

SparkQA · 2019-04-11T15:16:37Z

Test build #104510 has finished for PR 21586 at commit cde33ca.

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

SparkQA · 2019-04-11T18:00:27Z

Test build #104518 has finished for PR 21586 at commit 93f43ad.

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

SparkQA · 2019-04-15T18:10:20Z

Test build #104589 has finished for PR 21586 at commit 13a7846.

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

cloud-fan · 2019-05-15T06:02:10Z

retest this please

SparkQA · 2019-05-15T07:05:01Z

Test build #105401 has finished for PR 21586 at commit 7c6d9a8.

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

SparkQA · 2019-05-15T07:05:01Z

Test build #105398 has finished for PR 21586 at commit 13a7846.

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

SparkQA · 2019-05-15T07:05:02Z

Test build #105400 has finished for PR 21586 at commit a199906.

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

SparkQA · 2019-05-15T10:44:37Z

Test build #105408 has finished for PR 21586 at commit cf8d05f.

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

gatorsmile · 2019-05-16T18:34:22Z

+ * precision lose or possible runtime failures. For example, long -> int, string -> int are not
+ * up-cast.
 */
- def mayTruncate(from: DataType, to: DataType): Boolean = (from, to) match {


I know this will be ugly, but I still prefer to adding a conf for falling back to the original behavior.

SparkQA · 2019-05-17T15:57:26Z

Test build #105490 has finished for PR 21586 at commit 14e3de3.

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

SparkQA · 2019-05-17T17:08:59Z

Test build #105486 has finished for PR 21586 at commit e558ad4.

This patch fails Spark unit tests.
This patch does not merge cleanly.
This patch adds no public classes.

SparkQA · 2019-05-20T07:05:03Z

Test build #105551 has finished for PR 21586 at commit a638a53.

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

cloud-fan · 2019-05-21T06:24:12Z

retest this please

gatorsmile

LGTM

SparkQA · 2019-05-21T07:05:02Z

Test build #105598 has finished for PR 21586 at commit a638a53.

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

cloud-fan · 2019-05-21T13:25:41Z

retest this please

SparkQA · 2019-05-21T16:54:23Z

Test build #105618 has finished for PR 21586 at commit a638a53.

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

cloud-fan · 2019-05-22T03:36:08Z

thanks, merging to master!

dongjoon-hyun · 2019-05-22T04:30:42Z

cc @dbtsai

manuzhang · 2022-02-14T10:23:20Z

 need to specify a value with units like "30s" now, to avoid being interpreted as milliseconds; otherwise, 
 the extremely short interval that results will likely cause applications to fail.

+ - When turning a Dataset to another Dataset, Spark will up cast the fields in the original Dataset to the type of corresponding fields in the target DataSet. In version 2.4 and earlier, this up cast is not very strict, e.g. `Seq("str").toDS.as[Int]` fails, but `Seq("str").toDS.as[Boolean]` works and throw NPE during execution. In Spark 3.0, the up cast is stricter and turning String into something else is not allowed, i.e. `Seq("str").toDS.as[Boolean]` will fail during analysis.


@cloud-fan shall we mention setting spark.sql.legacy.looseUpcast=true to preserve old behavior?

Yeah we should

are you interested in creating a PR (with a new JIRA) to fix the doc? 2.4 is EOL but I think it's good to fix the docs anyway.

Sure. I will create one.

#35519 has been created.

viirya reviewed Jun 19, 2018
View reviewed changes

maropu reviewed Jun 19, 2018
View reviewed changes

viirya reviewed Jun 19, 2018
View reviewed changes

cloud-fan mentioned this pull request Jul 4, 2018
[SPARK-22384][SQL][followup] Refine partition pruning when attribute is wrapped in Cast #21712
Closed

cloud-fan mentioned this pull request Jul 5, 2018
[SPARK-24251][SQL] Add AppendData logical plan. #21305
Closed

cloud-fan force-pushed the cast branch 2 times, most recently from cde33ca to 2a64d59Compare April 11, 2019 11:39

Upcast should not allow casting from string to other types
93f43ad

cloud-fan force-pushed the cast branch from 2a64d59 to 93f43adCompare April 11, 2019 14:32

fix a case
13a7846

cloud-fan added 3 commits May 15, 2019 14:11

improve comment
a199906

update view as well
7c6d9a8

support complex types
cf8d05f

gatorsmile reviewed May 16, 2019
View reviewed changes

add a legacy config
e558ad4

Merge branch 'master' into cast
14e3de3

Merge branch 'master' into cast
a638a53

gatorsmile reviewed May 21, 2019
View reviewed changes

cloud-fan closed this in 03c9e8aMay 22, 2019

manuzhang reviewed Feb 14, 2022
View reviewed changes

Uh oh!

Conversation

cloud-fan commented Jun 18, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

cloud-fan commented Jun 18, 2018

Uh oh!

SparkQA commented Jun 19, 2018

Uh oh!

maropu commented Jun 19, 2018

Uh oh!

viiryaJun 19, 2018• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fanApr 11, 2019• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 19, 2018

Uh oh!

SparkQA commented Aug 3, 2018

Uh oh!

SparkQA commented Apr 11, 2019

Uh oh!

SparkQA commented Apr 11, 2019

Uh oh!

SparkQA commented Apr 11, 2019

Uh oh!

SparkQA commented Apr 15, 2019

Uh oh!

cloud-fan commented May 15, 2019

Uh oh!

SparkQA commented May 15, 2019

Uh oh!

SparkQA commented May 15, 2019

Uh oh!

SparkQA commented May 15, 2019

Uh oh!

SparkQA commented May 15, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 17, 2019

Uh oh!

SparkQA commented May 17, 2019

Uh oh!

SparkQA commented May 20, 2019

Uh oh!

cloud-fan commented May 21, 2019

Uh oh!

gatorsmile left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 21, 2019

Uh oh!

cloud-fan commented May 21, 2019

Uh oh!

SparkQA commented May 21, 2019

Uh oh!

cloud-fan commented May 22, 2019

Uh oh!

dongjoon-hyun commented May 22, 2019

Uh oh!

manuzhangFeb 14, 2022• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Jun 18, 2018•
edited
Loading

viiryaJun 19, 2018•
edited
Loading

cloud-fanApr 11, 2019•
edited
Loading

manuzhangFeb 14, 2022•
edited
Loading