[SPARK-30592][SQL] Interval support for csv and json funtions #27317

yaooqinn · 2020-01-22T08:17:33Z

What changes were proposed in this pull request?

In this PR, I'd propose to fully support interval for the CSV and JSON functions.

On one hand, CSV and JSON records consist of string values, in the cast logic, we can cast string from/to interval now, so we can make those functions support intervals easily.

Before this change we can only use this as a workaround.

SELECT cast(from_csv('1, 1 day', 'a INT, b string').b as interval)
struct<CAST(from_csv(1, 1 day).b AS INTERVAL):interval>
1 days

On the other hand, we ban reading or writing intervals from CSV and JSON files. To directly read and write with external json/csv storage, you still need explicit cast, e.g.

spark.read.schema("a string").json("a.json").selectExpr("cast(a as interval)").show
+------+
|     a|
+------+
|1 days|
+------+

Why are the changes needed?

for interval's future-proofing purpose

Does this PR introduce any user-facing change?

yes, the to_json/from_json function can deal with intervals now. e.g.
for from_json there is no such use case because we do not support a interval
for to_json, we can use interval values now

before

SELECT to_json(map('a', interval 25 month 100 day 130 minute));
Error in query: cannot resolve 'to_json(map('a', INTERVAL '2 years 1 months 100 days 2 hours 10 minutes'))' due to data type mismatch: Unable to convert column a of type interval to JSON.; line 1 pos 7;
'Project [unresolvedalias(to_json(map(a, 2 years 1 months 100 days 2 hours 10 minutes), Some(Asia/Shanghai)), None)]
+- OneRowRelation

after

SELECT to_json(map('a', interval 25 month 100 day 130 minute))
{"a":"2 years 1 months 100 days 2 hours 10 minutes"}

How was this patch tested?

add ut

SELECT cast(from_csv('1, 1 day', 'a INT, b string').b as interval) struct<CAST(from_csv(1, 1 day).b AS INTERVAL):interval> 1 days

This reverts commit d0dc6cf.

This reverts commit aa7742c.

…tions" This reverts commit 4f11feb.

MaxGekk

Could you add a test for spark.sql.legacy.to_json.verifyMapKeyType.enabled is false to be sure that the config works.

yaooqinn · 2020-01-22T11:11:01Z

Thanks, @MaxGekk on second thought, we should support these functions to deal with intervals as we only need to prohibit interval from external storage. So I have removed that conf

cloud-fan · 2020-01-22T11:41:18Z

...catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/JsonExpressionsSuite.scala

-    val mapType1 = MapType(CalendarIntervalType, IntegerType)
-    val schema1 = StructType(StructField("a", mapType1) :: Nil)
-    val struct1 = Literal.create(null, schema1)
+  test("from_json - interval support") {


nit: can we follow csv test and test from/to in one test case?

cloud-fan · 2020-01-22T11:47:12Z

sql/core/src/test/scala/org/apache/spark/sql/JsonFunctionsSuite.scala

@@ -222,33 +222,24 @@ class JsonFunctionsSuite extends QueryTest with SharedSparkSession {
      Row("""{"_1":"26/08/2015 18:00"}""") :: Nil)
  }

-  test("to_json - key types of map don't matter") {


I think we can remove it now. There is no unsupported types anymore. JSON supports all the types.

MaxGekk · 2020-01-22T11:56:15Z

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/json/JacksonParser.scala

+      parseJsonToken[CalendarInterval](parser, dataType) {
+        case VALUE_STRING =>
+          IntervalUtils.safeStringToInterval(UTF8String.fromString(parser.getText))
+    }


nit: shift } to the right, it belongs to parseJsonToken not to case

SparkQA · 2020-01-22T12:30:54Z

Test build #117222 has finished for PR 27317 at commit aa7742c.

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

SparkQA · 2020-01-22T12:41:27Z

Test build #117223 has finished for PR 27317 at commit d0dc6cf.

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

SparkQA · 2020-01-22T15:03:28Z

Test build #117230 has finished for PR 27317 at commit 9cb10ff.

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

SparkQA · 2020-01-22T15:04:43Z

Test build #117232 has finished for PR 27317 at commit 4d0df26.

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

SparkQA · 2020-01-22T16:22:10Z

Test build #117235 has finished for PR 27317 at commit 5a32fed.

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

cloud-fan · 2020-01-22T16:42:00Z

thanks, merging to master!

gatorsmile · 2020-02-09T00:33:04Z

sql/core/src/test/resources/sql-tests/inputs/interval.sql

+SELECT from_csv('1, 1 day', 'a INT, b interval');
+SELECT to_csv(named_struct('a', interval 32 month, 'b', interval 70 minute));
+SELECT from_json('{"a":"1 days"}', 'a interval');
+SELECT to_json(map('a', interval 25 month 100 day 130 minute));


@yaooqinn Could you add round-trip test cases?

from_csv(to_csv(....))

to_csv(from_csv(...))

from_json(to_json(....))

to_json(from_json(....))

ok, I'll make a followup.

gatorsmile · 2020-02-09T00:56:39Z

@yaooqinn Does this PR introduce any user-facing change? does not include the changes made in CSV. We need to update the PR description when you updated the code.

yaooqinn · 2020-02-09T11:46:10Z

@yaooqinn Does this PR introduce any user-facing change? does not include the changes made in CSV. We need to update the PR description when you updated the code.

The changes in CSV are not user-facing

### What changes were proposed in this pull request? Add round-trip tests for CSV and JSON functions as #27317 (comment) asked. ### Why are the changes needed? improve test coverage ### Does this PR introduce any user-facing change? no ### How was this patch tested? add uts Closes #27510 from yaooqinn/SPARK-30592-F. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org>

### What changes were proposed in this pull request? Add round-trip tests for CSV and JSON functions as #27317 (comment) asked. ### Why are the changes needed? improve test coverage ### Does this PR introduce any user-facing change? no ### How was this patch tested? add uts Closes #27510 from yaooqinn/SPARK-30592-F. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org> (cherry picked from commit 58b9ca1) Signed-off-by: HyukjinKwon <gurwls223@apache.org>

### What changes were proposed in this pull request? Add round-trip tests for CSV and JSON functions as apache#27317 (comment) asked. ### Why are the changes needed? improve test coverage ### Does this PR introduce any user-facing change? no ### How was this patch tested? add uts Closes apache#27510 from yaooqinn/SPARK-30592-F. Authored-by: Kent Yao <yaooqinn@hotmail.com> Signed-off-by: HyukjinKwon <gurwls223@apache.org>

yaooqinn added 3 commits January 22, 2020 15:56

[SPARK-30592][SQL] Interval type checker for csv and json funtions

4f11feb

SELECT cast(from_csv('1, 1 day', 'a INT, b string').b as interval) struct<CAST(from_csv(1, 1 day).b AS INTERVAL):interval> 1 days

clean tests

aa7742c

legacy config

d0dc6cf

yaooqinn requested a review from cloud-fan January 22, 2020 08:30

yaooqinn added 3 commits January 22, 2020 17:42

Revert "legacy config"

ee82ac6

This reverts commit d0dc6cf.

Revert "clean tests"

0e7f7ba

This reverts commit aa7742c.

Revert "[SPARK-30592][SQL] Interval type checker for csv and json fun…

9a66cec

…tions" This reverts commit 4f11feb.

MaxGekk reviewed Jan 22, 2020

View reviewed changes

yaooqinn added 2 commits January 22, 2020 18:42

support not ban

9cb10ff

nit

4d0df26

cloud-fan reviewed Jan 22, 2020

View reviewed changes

yaooqinn changed the title ~~[SPARK-30592][SQL] Interval type checker for csv and json funtions~~ [SPARK-30592][SQL] Support Intervals for csv and json funtions Jan 22, 2020

yaooqinn changed the title ~~[SPARK-30592][SQL] Support Intervals for csv and json funtions~~ [SPARK-30592][SQL] Interval support for csv and json funtions Jan 22, 2020

MaxGekk reviewed Jan 22, 2020

View reviewed changes

nit

5a32fed

cloud-fan closed this in 8e280ce Jan 22, 2020

dongjoon-hyun added the SQL label Feb 5, 2020

gatorsmile reviewed Feb 9, 2020

View reviewed changes

yaooqinn mentioned this pull request Feb 9, 2020

[SPARK-30592][SQL][FOLLOWUP] Add some round-trip test cases #27510

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-30592][SQL] Interval support for csv and json funtions #27317

[SPARK-30592][SQL] Interval support for csv and json funtions #27317

yaooqinn commented Jan 22, 2020 •

edited

MaxGekk left a comment

yaooqinn commented Jan 22, 2020

cloud-fan Jan 22, 2020

yaooqinn Jan 22, 2020

cloud-fan Jan 22, 2020

MaxGekk Jan 22, 2020

yaooqinn Jan 22, 2020

SparkQA commented Jan 22, 2020

SparkQA commented Jan 22, 2020

SparkQA commented Jan 22, 2020

SparkQA commented Jan 22, 2020

SparkQA commented Jan 22, 2020

cloud-fan commented Jan 22, 2020

gatorsmile Feb 9, 2020

yaooqinn Feb 9, 2020

gatorsmile commented Feb 9, 2020

yaooqinn commented Feb 9, 2020

[SPARK-30592][SQL] Interval support for csv and json funtions #27317

[SPARK-30592][SQL] Interval support for csv and json funtions #27317

Conversation

yaooqinn commented Jan 22, 2020 • edited

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

before

after

How was this patch tested?

MaxGekk left a comment

Choose a reason for hiding this comment

yaooqinn commented Jan 22, 2020

cloud-fan Jan 22, 2020

Choose a reason for hiding this comment

yaooqinn Jan 22, 2020

Choose a reason for hiding this comment

cloud-fan Jan 22, 2020

Choose a reason for hiding this comment

MaxGekk Jan 22, 2020

Choose a reason for hiding this comment

yaooqinn Jan 22, 2020

Choose a reason for hiding this comment

SparkQA commented Jan 22, 2020

SparkQA commented Jan 22, 2020

SparkQA commented Jan 22, 2020

SparkQA commented Jan 22, 2020

SparkQA commented Jan 22, 2020

cloud-fan commented Jan 22, 2020

gatorsmile Feb 9, 2020

Choose a reason for hiding this comment

yaooqinn Feb 9, 2020

Choose a reason for hiding this comment

gatorsmile commented Feb 9, 2020

yaooqinn commented Feb 9, 2020

yaooqinn commented Jan 22, 2020 •

edited