[SPARK-25945][SQL] Support locale while parsing date/timestamp from CSV/JSON by MaxGekk · Pull Request #22951 · apache/spark

MaxGekk · 2018-11-05T19:53:38Z

What changes were proposed in this pull request?

In the PR, I propose to add new option locale into CSVOptions/JSONOptions to make parsing date/timestamps in local languages possible. Currently the locale is hard coded to Locale.US.

How was this patch tested?

Added two tests for parsing a date from CSV/JSON - ноя 2018.

MaxGekk · 2018-11-05T20:04:13Z

I will update docs soon.

SparkQA · 2018-11-05T23:19:12Z

Test build #98489 has finished for PR 22951 at commit 41154bd.

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

MaxGekk · 2018-11-06T09:07:53Z

@HyukjinKwon @dongjoon-hyun Please, review the changes.

SparkQA · 2018-11-06T12:41:42Z

Test build #98507 has finished for PR 22951 at commit 93da760.

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

HyukjinKwon · 2018-11-07T03:33:03Z

Looks good. I or someone else should take a closer look before getting this in.

dongjoon-hyun · 2018-11-07T04:31:10Z

Could you rebase this once again, @MaxGekk ?

# Conflicts: # sql/core/src/test/scala/org/apache/spark/sql/CsvFunctionsSuite.scala

SparkQA · 2018-11-07T08:05:02Z

Test build #98541 has finished for PR 22951 at commit 6ab8501.

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

HyukjinKwon · 2018-11-07T08:23:13Z

retest this please

SparkQA · 2018-11-07T12:05:43Z

Test build #98543 has finished for PR 22951 at commit 6ab8501.

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

SparkQA · 2018-11-07T23:20:54Z

Test build #98567 has finished for PR 22951 at commit 759bca6.

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

dongjoon-hyun

+1, LGTM.

dongjoon-hyun · 2018-11-08T05:50:55Z

Could you take a look once more, @HyukjinKwon ?

HyukjinKwon · 2018-11-08T06:24:34Z

OMG, what does ноя 2018 mean BTW? haha

HyukjinKwon · 2018-11-08T06:27:55Z

 the default value, empty string.
+ :param locale: sets a locale as language tag in IETF BCP 47 format. If None is set,
+ it uses the default value, ``en-US``. For instance, ``locale`` is used while
+ parsing dates and timestamps.


I think ideally we should apply to decimal parsing too actually. But yea we can leave it separate.

It seems parsing decimals using locale will be slightly tricky in JSON case because we leave this to Jackson by calling its method getCurrentToken and getDecimalValue, and I haven't found how to pass locale to it. Probably we will need a custom deserialiser?
In the CSV case, it should be easier since we convert strings ourselves. I will try to do that for CSV first of all when this PR be merged.

Here is the PR for parsing decimals from CSV: #22979

HyukjinKwon · 2018-11-08T06:33:34Z

 maxCharsPerColumn=None, maxMalformedLogPerPartition=None, mode=None,
 columnNameOfCorruptRecord=None, multiLine=None, charToEscapeQuoteEscaping=None,
- samplingRatio=None, enforceSchema=None, emptyValue=None):
+ samplingRatio=None, enforceSchema=None, emptyValue=None, locale=None):


Let's add emptyValue in streaming.py in the same separate PR.

It seems it exists in streaming.py:
spark/python/pyspark/sql/streaming.py
Line 567 in 08c76b5
enforceSchema=None, emptyValue=None):

MaxGekk · 2018-11-08T07:31:36Z

OMG, what does ноя 2018 mean BTW? haha

It is 3 letters prefix of Ноябрь which is November in Russian.

# Conflicts: # sql/core/src/test/scala/org/apache/spark/sql/CsvFunctionsSuite.scala # sql/core/src/test/scala/org/apache/spark/sql/JsonFunctionsSuite.scala

SparkQA · 2018-11-08T08:05:01Z

Test build #98583 has finished for PR 22951 at commit 8834b4b.

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

MaxGekk · 2018-11-08T08:07:12Z

jenkins, retest this, please

SparkQA · 2018-11-08T09:43:50Z

Test build #98587 has finished for PR 22951 at commit 8834b4b.

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

MaxGekk · 2018-11-08T09:52:48Z

jenkins, retest this, please

SparkQA · 2018-11-08T11:34:52Z

Test build #98591 has finished for PR 22951 at commit 8834b4b.

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

HyukjinKwon · 2018-11-08T11:49:16Z

retest this please

SparkQA · 2018-11-08T15:21:07Z

Test build #98598 has finished for PR 22951 at commit 8834b4b.

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

HyukjinKwon · 2018-11-09T01:44:41Z

Merged to master.

HyukjinKwon · 2018-11-09T01:45:40Z

Actually let me leave a cc for @srowen. I remember we talked about it before.

…SV/JSON ## What changes were proposed in this pull request? In the PR, I propose to add new option `locale` into CSVOptions/JSONOptions to make parsing date/timestamps in local languages possible. Currently the locale is hard coded to `Locale.US`. ## How was this patch tested? Added two tests for parsing a date from CSV/JSON - `ноя 2018`. Closesapache#22951 from MaxGekk/locale. Authored-by: Maxim Gekk <max.gekk@gmail.com> Signed-off-by: hyukjinkwon <gurwls223@apache.org>

MaxGekk added 5 commits November 5, 2018 22:09

Added a test for from_csv
c71cd4f

lang -> langTag
e55a3d3

Test for from_json
83c6317

Added locale option for JSON and CSV
fa019ec

Fix imports
41154bd

MaxGekk added 3 commits November 6, 2018 11:23

Updating docs
7273d2c

Tests for from_json/from_csv
402b1a2

Fix imports
93da760

Merge remote-tracking branch 'origin/master' into locale
6ab8501
# Conflicts: # sql/core/src/test/scala/org/apache/spark/sql/CsvFunctionsSuite.scala

dongjoon-hyun reviewed Nov 7, 2018
View reviewed changes

Comment threadsql/core/src/test/scala/org/apache/spark/sql/CsvFunctionsSuite.scala Outdated

dongjoon-hyun reviewed Nov 7, 2018
View reviewed changes

Comment threadsql/core/src/test/scala/org/apache/spark/sql/JsonFunctionsSuite.scala Outdated

dongjoon-hyun reviewed Nov 7, 2018
View reviewed changes

Comment threadsql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CsvExpressionsSuite.scala Outdated

Addressing Dongjoon's review comments
759bca6

dongjoon-hyun approved these changes Nov 8, 2018
View reviewed changes

HyukjinKwon reviewed Nov 8, 2018
View reviewed changes

HyukjinKwon approved these changes Nov 8, 2018•
edited
Loading
View reviewed changes

HyukjinKwon reviewed Nov 8, 2018
View reviewed changes

Comment threadpython/pyspark/sql/readwriter.py

HyukjinKwon reviewed Nov 8, 2018
View reviewed changes

Comment threadpython/pyspark/sql/readwriter.py

HyukjinKwon reviewed Nov 8, 2018
View reviewed changes

MaxGekk added 2 commits November 8, 2018 10:36

Merge remote-tracking branch 'origin/master' into locale
0aa145a
# Conflicts: # sql/core/src/test/scala/org/apache/spark/sql/CsvFunctionsSuite.scala # sql/core/src/test/scala/org/apache/spark/sql/JsonFunctionsSuite.scala

Adding the locale parameter to json()/csv()
8834b4b

MaxGekk mentioned this pull request Nov 8, 2018
[SPARK-25977][SQL] Parsing decimals from CSV using locale #22979
Closed

asfgit closed this in 79551f5Nov 9, 2018

MaxGekk deleted the locale branch August 17, 2019 13:32

Uh oh!

Conversation

MaxGekk commented Nov 5, 2018

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

MaxGekk commented Nov 5, 2018

Uh oh!

SparkQA commented Nov 5, 2018

Uh oh!

MaxGekk commented Nov 6, 2018

Uh oh!

SparkQA commented Nov 6, 2018

Uh oh!

HyukjinKwon commented Nov 7, 2018

Uh oh!

dongjoon-hyun commented Nov 7, 2018

Uh oh!

SparkQA commented Nov 7, 2018

Uh oh!

HyukjinKwon commented Nov 7, 2018

Uh oh!

SparkQA commented Nov 7, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SparkQA commented Nov 7, 2018

Uh oh!

dongjoon-hyun left a comment

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Nov 8, 2018

Uh oh!

HyukjinKwon commented Nov 8, 2018

Uh oh!

HyukjinKwonNov 8, 2018

Choose a reason for hiding this comment

Uh oh!

MaxGekkNov 8, 2018

Choose a reason for hiding this comment

Uh oh!

MaxGekkNov 8, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HyukjinKwonNov 8, 2018

Choose a reason for hiding this comment

Uh oh!

MaxGekkNov 8, 2018

Choose a reason for hiding this comment

Uh oh!

MaxGekk commented Nov 8, 2018

Uh oh!

SparkQA commented Nov 8, 2018

Uh oh!

MaxGekk commented Nov 8, 2018

Uh oh!

SparkQA commented Nov 8, 2018

Uh oh!

MaxGekk commented Nov 8, 2018

Uh oh!

SparkQA commented Nov 8, 2018

Uh oh!

HyukjinKwon commented Nov 8, 2018

Uh oh!

SparkQA commented Nov 8, 2018

Uh oh!

HyukjinKwon commented Nov 9, 2018

Uh oh!

HyukjinKwon commented Nov 9, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development