[SPARK-25040][SQL] Empty string for non string types should be disallowed #22787

viirya · 2018-10-21T13:00:29Z

What changes were proposed in this pull request?

This takes over original PR at #22019. The original proposal is to have null for float and double types. Later a more reasonable proposal is to disallow empty strings. This patch adds logic to throw exception when finding empty strings for non string types.

How was this patch tested?

Added test.

viirya · 2018-10-21T13:00:37Z

cc @HyukjinKwon

HyukjinKwon · 2018-10-21T14:29:21Z

Yea looks good as we discussed. Should we maybe better update the migration guide too while we are here?

viirya · 2018-10-21T14:31:56Z

Yea, I do think so. I will update it.

SparkQA · 2018-10-21T16:45:33Z

Test build #97684 has finished for PR 22787 at commit 45aacaf.

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

docs/sql-migration-guide-upgrade.md

SparkQA · 2018-10-21T18:38:44Z

Test build #97685 has finished for PR 22787 at commit 589caf7.

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

HyukjinKwon · 2018-10-22T04:27:33Z

docs/sql-migration-guide-upgrade.md


  - In PySpark, when creating a `SparkSession` with `SparkSession.builder.getOrCreate()`, if there is an existing `SparkContext`, the builder was trying to update the `SparkConf` of the existing `SparkContext` with configurations specified to the builder, but the `SparkContext` is shared by all `SparkSession`s, so we should not update them. Since 3.0, the builder comes to not update the configurations. This is the same behavior as Java/Scala API in 2.3 and above. If you want to update them, you need to update them prior to creating a `SparkSession`.

+  - In Spark version 2.4 and earlier, the parser of JSON data source treats empty strings as null for some data types like `IntegerType`. For `FloatType` and `DoubleType`, it fails on empty strings and throws exceptions. Since Spark 3.0, we disallow empty strings and will throw exceptions for data types except for `StringType` and `BinaryType`.


personal preference .. : some data types such as IntegerType.

HyukjinKwon · 2018-10-22T05:10:49Z

github looks buggy for now. Let me clean up my comments if they got messed.

SparkQA · 2018-10-22T07:05:03Z

Test build #97690 has finished for PR 22787 at commit 589caf7.

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

SparkQA · 2018-10-22T07:05:04Z

Test build #97692 has finished for PR 22787 at commit bb117f2.

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

SparkQA · 2018-10-22T07:05:05Z

Test build #97695 has finished for PR 22787 at commit 589caf7.

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

SparkQA · 2018-10-22T07:05:05Z

Test build #97688 has finished for PR 22787 at commit bb117f2.

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

SparkQA · 2018-10-22T12:23:34Z

Test build #97762 has finished for PR 22787 at commit 589caf7.

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

SparkQA · 2018-10-22T12:25:40Z

Test build #97757 has finished for PR 22787 at commit bb117f2.

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

SparkQA · 2018-10-22T13:44:42Z

Test build #97832 has started for PR 22787 at commit c04ea64.

SparkQA · 2018-10-22T14:24:09Z

Test build #97769 has finished for PR 22787 at commit bb117f2.

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

SparkQA · 2018-10-22T14:25:02Z

Test build #97843 has started for PR 22787 at commit c04ea64.

SparkQA · 2018-10-22T14:26:59Z

Test build #97837 has finished for PR 22787 at commit bb117f2.

This patch fails to generate documentation.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2018-10-22T14:31:53Z

Test build #97775 has finished for PR 22787 at commit 589caf7.

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

SparkQA · 2018-10-22T14:31:57Z

Test build #97852 has started for PR 22787 at commit bb117f2.

SparkQA · 2018-10-22T14:44:25Z

Test build #97858 has started for PR 22787 at commit c04ea64.

HyukjinKwon · 2018-10-22T14:55:11Z

It's chaotic ...

SparkQA · 2018-10-22T15:09:55Z

Test build #97864 has started for PR 22787 at commit bb117f2.

SparkQA · 2018-10-22T16:14:46Z

Test build #97869 has finished for PR 22787 at commit c04ea64.

This patch fails to generate documentation.
This patch merges cleanly.
This patch adds no public classes.

viirya · 2018-10-22T16:29:56Z

Thanks @HyukjinKwon. I've fixed that and committed. But looks like Github is out of order now. The commit is now disappeared. Let's wait for Github backing to normal. :)

…

On Mon, Oct 22, 2018 at 12:50 PM Hyukjin Kwon ***@***.***> wrote: ***@***.**** approved this pull request. Yea, looks we should fix the comments. LGTM otherwise — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#22787 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAEM9_r0kwKh8oV3uNoOLB59LU_Kwtgnks5unU6KgaJpZM4Xycnv> .

viirya · 2018-10-23T00:20:44Z

retest this please.

viirya · 2018-10-23T00:21:26Z

Seems github is restored...

HyukjinKwon

LGTM

viirya · 2018-10-23T01:41:35Z

cc @cloud-fan

SparkQA · 2018-10-23T04:12:09Z

Test build #97884 has finished for PR 22787 at commit c04ea64.

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

HyukjinKwon · 2018-10-23T05:43:56Z

Merged to master.

viirya · 2018-10-23T06:13:38Z

Thanks @HyukjinKwon @dongjoon-hyun

…owed ## What changes were proposed in this pull request? This takes over original PR at apache#22019. The original proposal is to have null for float and double types. Later a more reasonable proposal is to disallow empty strings. This patch adds logic to throw exception when finding empty strings for non string types. ## How was this patch tested? Added test. Closes apache#22787 from viirya/SPARK-25040. Authored-by: Liang-Chi Hsieh <viirya@gmail.com> Signed-off-by: hyukjinkwon <gurwls223@apache.org>

cloud-fan · 2020-02-04T08:24:54Z

can we add a legacy config for it?

viirya · 2020-02-04T09:07:54Z

@cloud-fan Ok. I can add a legacy config for this.

cloud-fan · 2020-02-04T10:09:51Z

@viirya thanks!

…ings for certain types in json parser ### What changes were proposed in this pull request? This is a follow-up for #22787. In #22787 we disallowed empty strings for json parser except for string and binary types. This follow-up adds a legacy config for restoring previous behavior of allowing empty string. ### Why are the changes needed? Adding a legacy config to make migration easy for Spark users. ### Does this PR introduce any user-facing change? Yes. If set this legacy config to true, the users can restore previous behavior prior to Spark 3.0.0. ### How was this patch tested? Unit test. Closes #27456 from viirya/SPARK-25040-followup. Lead-authored-by: Liang-Chi Hsieh <liangchi@uber.com> Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

…ings for certain types in json parser ### What changes were proposed in this pull request? This is a follow-up for #22787. In #22787 we disallowed empty strings for json parser except for string and binary types. This follow-up adds a legacy config for restoring previous behavior of allowing empty string. ### Why are the changes needed? Adding a legacy config to make migration easy for Spark users. ### Does this PR introduce any user-facing change? Yes. If set this legacy config to true, the users can restore previous behavior prior to Spark 3.0.0. ### How was this patch tested? Unit test. Closes #27456 from viirya/SPARK-25040-followup. Lead-authored-by: Liang-Chi Hsieh <liangchi@uber.com> Co-authored-by: Liang-Chi Hsieh <viirya@gmail.com> Signed-off-by: Dongjoon Hyun <dhyun@apple.com> (cherry picked from commit 7631275) Signed-off-by: Dongjoon Hyun <dhyun@apple.com>

Disallow empty strings on non string types.

45aacaf

viirya force-pushed the SPARK-25040 branch from 060622e to 3136014 Compare October 21, 2018 14:55

Update migration guide.

589caf7

viirya force-pushed the SPARK-25040 branch from 3136014 to 589caf7 Compare October 21, 2018 14:56

dongjoon-hyun reviewed Oct 21, 2018

View reviewed changes

docs/sql-migration-guide-upgrade.md Outdated Show resolved Hide resolved

Address comment.

bb117f2

HyukjinKwon approved these changes Oct 22, 2018 •

edited

Loading

View reviewed changes

HyukjinKwon reviewed Oct 22, 2018

View reviewed changes

HyukjinKwon approved these changes Oct 22, 2018 •

edited

Loading

View reviewed changes

Address comment.

c04ea64

HyukjinKwon approved these changes Oct 23, 2018

View reviewed changes

asfgit closed this in 03e82e3 Oct 23, 2018

viirya mentioned this pull request Feb 4, 2020

[SPARK-25040][SQL][FOLLOWUP] Add legacy config for allowing empty strings for certain types in json parser #27456

Closed

viirya deleted the SPARK-25040 branch December 27, 2023 18:22


		- In PySpark, when creating a `SparkSession` with `SparkSession.builder.getOrCreate()`, if there is an existing `SparkContext`, the builder was trying to update the `SparkConf` of the existing `SparkContext` with configurations specified to the builder, but the `SparkContext` is shared by all `SparkSession`s, so we should not update them. Since 3.0, the builder comes to not update the configurations. This is the same behavior as Java/Scala API in 2.3 and above. If you want to update them, you need to update them prior to creating a `SparkSession`.

		- In Spark version 2.4 and earlier, the parser of JSON data source treats empty strings as null for some data types like `IntegerType`. For `FloatType` and `DoubleType`, it fails on empty strings and throws exceptions. Since Spark 3.0, we disallow empty strings and will throw exceptions for data types except for `StringType` and `BinaryType`.

[SPARK-25040][SQL] Empty string for non string types should be disallowed #22787

[SPARK-25040][SQL] Empty string for non string types should be disallowed #22787

Uh oh!

Conversation

viirya commented Oct 21, 2018

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

viirya commented Oct 21, 2018

Uh oh!

HyukjinKwon commented Oct 21, 2018

Uh oh!

viirya commented Oct 21, 2018

Uh oh!

SparkQA commented Oct 21, 2018

Uh oh!

Uh oh!

SparkQA commented Oct 21, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HyukjinKwon Oct 22, 2018

Choose a reason for hiding this comment

Uh oh!

viirya Oct 22, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

HyukjinKwon commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

HyukjinKwon commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

SparkQA commented Oct 22, 2018

Uh oh!

viirya commented Oct 22, 2018 via email

Uh oh!

viirya commented Oct 23, 2018

Uh oh!

viirya commented Oct 23, 2018

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

viirya commented Oct 23, 2018

Uh oh!

SparkQA commented Oct 23, 2018

Uh oh!

HyukjinKwon commented Oct 23, 2018

Uh oh!

viirya commented Oct 23, 2018

Uh oh!