[SPARK-28454][PYTHON] Validate LongType in `createDataFrame(verifySchema=True)` by simplylizz · Pull Request #25117 · apache/spark

simplylizz · 2019-07-11T13:55:39Z

What changes were proposed in this pull request?

Add missing validation for LongType in pyspark.sql.types._make_type_verifier.

How was this patch tested?

Doctests / unittests / manual tests.

Unpatched version:

In [23]: s.createDataFrame([{'x': 1 << 64}], StructType([StructField('x', LongType())])).collect()
Out[23]: [Row(x=None)]

Patched:

In [5]: s.createDataFrame([{'x': 1 << 64}], StructType([StructField('x', LongType())])).collect()
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-5-c1740fcadbf9> in <module>
----> 1 s.createDataFrame([{'x': 1 << 64}], StructType([StructField('x', LongType())])).collect()

/usr/local/lib/python3.5/site-packages/pyspark/sql/session.py in createDataFrame(self, data, schema, samplingRatio, verifySchema)
    689             rdd, schema = self._createFromRDD(data.map(prepare), schema, samplingRatio)
    690         else:
--> 691             rdd, schema = self._createFromLocal(map(prepare, data), schema)
    692         jrdd = self._jvm.SerDeUtil.toJavaArray(rdd._to_java_object_rdd())
    693         jdf = self._jsparkSession.applySchemaToPythonRDD(jrdd.rdd(), schema.json())

/usr/local/lib/python3.5/site-packages/pyspark/sql/session.py in _createFromLocal(self, data, schema)
    405         # make sure data could consumed multiple times
    406         if not isinstance(data, list):
--> 407             data = list(data)
    408
    409         if schema is None or isinstance(schema, (list, tuple)):

/usr/local/lib/python3.5/site-packages/pyspark/sql/session.py in prepare(obj)
    671
    672             def prepare(obj):
--> 673                 verify_func(obj)
    674                 return obj
    675         elif isinstance(schema, DataType):

/usr/local/lib/python3.5/site-packages/pyspark/sql/types.py in verify(obj)
   1427     def verify(obj):
   1428         if not verify_nullability(obj):
-> 1429             verify_value(obj)
   1430
   1431     return verify

/usr/local/lib/python3.5/site-packages/pyspark/sql/types.py in verify_struct(obj)
   1397             if isinstance(obj, dict):
   1398                 for f, verifier in verifiers:
-> 1399                     verifier(obj.get(f))
   1400             elif isinstance(obj, Row) and getattr(obj, "__from_dict__", False):
   1401                 # the order in obj could be different than dataType.fields

/usr/local/lib/python3.5/site-packages/pyspark/sql/types.py in verify(obj)
   1427     def verify(obj):
   1428         if not verify_nullability(obj):
-> 1429             verify_value(obj)
   1430
   1431     return verify

/usr/local/lib/python3.5/site-packages/pyspark/sql/types.py in verify_long(obj)
   1356             if obj < -9223372036854775808 or obj > 9223372036854775807:
   1357                 raise ValueError(
-> 1358                     new_msg("object of LongType out of range, got: %s" % obj))
   1359
   1360         verify_value = verify_long

ValueError: field x: object of LongType out of range, got: 18446744073709551616

HyukjinKwon · 2019-07-15T04:16:50Z

Can you show the output before / after? Also, please file a JIRA. See https://spark.apache.org/contributing.html

HyukjinKwon · 2019-07-15T04:17:03Z

ok to test

SparkQA · 2019-07-15T04:35:37Z

Test build #107661 has finished for PR 25117 at commit 1d8761a.

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

simplylizz · 2019-07-19T22:28:19Z

Can you show the output before / after?

Could you clarify what kind of output do you want to see? Test results or what?

SparkQA · 2019-07-19T22:42:21Z

Test build #107928 has finished for PR 25117 at commit 87bc45c.

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

SparkQA · 2019-07-20T08:25:16Z

Test build #107936 has finished for PR 25117 at commit 97771d1.

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

HyukjinKwon · 2019-07-26T05:08:06Z

createDataFrame with verifySchema enabled where long exceeds the range you specified.

simplylizz · 2019-07-27T13:04:39Z

Unpatched version:

In [23]: s.createDataFrame([{'x': 1 << 64}], StructType([StructField('x', LongType())])).collect()
Out[23]: [Row(x=None)]

Patched:

In [5]: s.createDataFrame([{'x': 1 << 64}], StructType([StructField('x', LongType())])).collect()
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-5-c1740fcadbf9> in <module>
----> 1 s.createDataFrame([{'x': 1 << 64}], StructType([StructField('x', LongType())])).collect()

/usr/local/lib/python3.5/site-packages/pyspark/sql/session.py in createDataFrame(self, data, schema, samplingRatio, verifySchema)
    689             rdd, schema = self._createFromRDD(data.map(prepare), schema, samplingRatio)
    690         else:
--> 691             rdd, schema = self._createFromLocal(map(prepare, data), schema)
    692         jrdd = self._jvm.SerDeUtil.toJavaArray(rdd._to_java_object_rdd())
    693         jdf = self._jsparkSession.applySchemaToPythonRDD(jrdd.rdd(), schema.json())

/usr/local/lib/python3.5/site-packages/pyspark/sql/session.py in _createFromLocal(self, data, schema)
    405         # make sure data could consumed multiple times
    406         if not isinstance(data, list):
--> 407             data = list(data)
    408
    409         if schema is None or isinstance(schema, (list, tuple)):

/usr/local/lib/python3.5/site-packages/pyspark/sql/session.py in prepare(obj)
    671
    672             def prepare(obj):
--> 673                 verify_func(obj)
    674                 return obj
    675         elif isinstance(schema, DataType):

/usr/local/lib/python3.5/site-packages/pyspark/sql/types.py in verify(obj)
   1427     def verify(obj):
   1428         if not verify_nullability(obj):
-> 1429             verify_value(obj)
   1430
   1431     return verify

/usr/local/lib/python3.5/site-packages/pyspark/sql/types.py in verify_struct(obj)
   1397             if isinstance(obj, dict):
   1398                 for f, verifier in verifiers:
-> 1399                     verifier(obj.get(f))
   1400             elif isinstance(obj, Row) and getattr(obj, "__from_dict__", False):
   1401                 # the order in obj could be different than dataType.fields

/usr/local/lib/python3.5/site-packages/pyspark/sql/types.py in verify(obj)
   1427     def verify(obj):
   1428         if not verify_nullability(obj):
-> 1429             verify_value(obj)
   1430
   1431     return verify

/usr/local/lib/python3.5/site-packages/pyspark/sql/types.py in verify_long(obj)
   1356             if obj < -9223372036854775808 or obj > 9223372036854775807:
   1357                 raise ValueError(
-> 1358                     new_msg("object of LongType out of range, got: %s" % obj))
   1359
   1360         verify_value = verify_long

ValueError: field x: object of LongType out of range, got: 18446744073709551616

HyukjinKwon

Can you post the comment in PR description and update migration guide (https://github.com/apache/spark/blob/master/docs/sql-migration-guide-upgrade.md#upgrading-from-spark-sql-24-to-30) as well?

We can mention like in spark 3.0, it verifies long type when creating a dataframe via spark.createDataFrame. Previously long type was not verified and resulted in null. To keep this behavior, set verifySchema=False.

simplylizz · 2019-07-31T18:29:55Z

Done.

SparkQA · 2019-07-31T19:15:00Z

Test build #108489 has finished for PR 25117 at commit fde0a90.

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

SparkQA · 2019-07-31T20:56:13Z

Test build #108488 has finished for PR 25117 at commit 080431d.

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

simplylizz · 2019-08-07T09:10:01Z

@HyukjinKwon any action required from my side?

It seems that tests are just flapping. They are failing in unrelated parts of code which I didn't touch.

HyukjinKwon · 2019-08-07T09:25:56Z

retest this please

docs/sql-migration-guide-upgrade.md

HyukjinKwon

Looks good otherwise.

SparkQA · 2019-08-07T10:05:25Z

Test build #108759 has finished for PR 25117 at commit fde0a90.

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

SparkQA · 2019-08-07T10:32:44Z

Test build #108761 has finished for PR 25117 at commit 1f02d0c.

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

HyukjinKwon · 2019-08-08T02:46:55Z

Merged to master.

HyukjinKwon · 2019-08-08T02:48:35Z

Thanks for the contribution, @simplylizz

simplylizz changed the title ~~[Python] Validate LongType in _make_type_verifier~~ [SPARK-28454][Python] Validate LongType in _make_type_verifier Jul 19, 2019

dongjoon-hyun added the PYSPARK label Jul 27, 2019

HyukjinKwon reviewed Jul 29, 2019

View reviewed changes

HyukjinKwon changed the title ~~[SPARK-28454][Python] Validate LongType in _make_type_verifier~~ [SPARK-28454][PYTHON] Validate LongType in _make_type_verifier Jul 29, 2019

HyukjinKwon changed the title ~~[SPARK-28454][PYTHON] Validate LongType in _make_type_verifier~~ [SPARK-28454][PYTHON] Validate LongType in createDataFrame(verifySchema=True) Jul 29, 2019

simplylizz added 4 commits July 31, 2019 20:27

Validate LongType in _make_type_verifier

831056f

Fix tests

71b7b36

More consistent code style

ffa9b11

Update docs

fde0a90

HyukjinKwon reviewed Aug 7, 2019

View reviewed changes

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

HyukjinKwon reviewed Aug 7, 2019

View reviewed changes

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

HyukjinKwon approved these changes Aug 7, 2019

View reviewed changes

Review fix

1f02d0c

HyukjinKwon closed this in bda5b51 Aug 8, 2019

Comments

Conversation

simplylizz commented Jul 11, 2019 • 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!

HyukjinKwon commented Jul 15, 2019

Uh oh!

HyukjinKwon commented Jul 15, 2019

Uh oh!

SparkQA commented Jul 15, 2019

Uh oh!

simplylizz commented Jul 19, 2019

Uh oh!

SparkQA commented Jul 19, 2019

Uh oh!

SparkQA commented Jul 20, 2019

Uh oh!

HyukjinKwon commented Jul 26, 2019

Uh oh!

simplylizz commented Jul 27, 2019

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

simplylizz commented Jul 31, 2019

Uh oh!

SparkQA commented Jul 31, 2019

Uh oh!

SparkQA commented Jul 31, 2019

Uh oh!

simplylizz commented Aug 7, 2019

Uh oh!

HyukjinKwon commented Aug 7, 2019

Uh oh!

Uh oh!

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Aug 7, 2019

Uh oh!

SparkQA commented Aug 7, 2019

Uh oh!

HyukjinKwon commented Aug 8, 2019

Uh oh!

HyukjinKwon commented Aug 8, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

simplylizz commented Jul 11, 2019 •

edited

Loading