[SPARK-26227][R] from_[csv|json] should accept schema_of_[csv|json] in R API #23184

HyukjinKwon · 2018-11-30T03:49:25Z

What changes were proposed in this pull request?

1. Document from_csv(..., schema_of_csv(...)) support:

csv <- "Amsterdam,2018"
df <- sql(paste0("SELECT '", csv, "' as csv"))
head(select(df, from_csv(df$csv, schema_of_csv(csv))))

    from_csv(csv)
1 Amsterdam, 2018

2. Allow from_json(..., schema_of_json(...))

Before:

df2 <- sql("SELECT named_struct('name', 'Bob') as people")
df2 <- mutate(df2, people_json = to_json(df2$people))
head(select(df2, from_json(df2$people_json, schema_of_json(head(df2)$people_json))))

Error in (function (classes, fdef, mtable)  :
  unable to find an inherited method for function ‘from_json’ for signature ‘"Column", "Column"’

After:

df2 <- sql("SELECT named_struct('name', 'Bob') as people")
df2 <- mutate(df2, people_json = to_json(df2$people))
head(select(df2, from_json(df2$people_json, schema_of_json(head(df2)$people_json))))

  from_json(people_json)
1                    Bob

3. (While I'm here) Allow structType as schema for from_csv support to match with from_json.

Before:

csv <- "Amsterdam,2018"
df <- sql(paste0("SELECT '", csv, "' as csv"))
head(select(df, from_csv(df$csv, structType("city STRING, year INT"))))

Error in (function (classes, fdef, mtable)  :
  unable to find an inherited method for function ‘from_csv’ for signature ‘"Column", "structType"’

After:

csv <- "Amsterdam,2018"
df <- sql(paste0("SELECT '", csv, "' as csv"))
head(select(df, from_csv(df$csv, structType("city STRING, year INT"))))

    from_csv(csv)
1 Amsterdam, 2018

How was this patch tested?

Manually tested and unittests were added.

HyukjinKwon · 2018-11-30T03:54:34Z

cc @felixcheung, @viirya and @MaxGekk

SparkQA · 2018-11-30T07:26:57Z

Test build #99494 has finished for PR 23184 at commit 8877837.

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

R/pkg/R/functions.R

sql/core/src/main/scala/org/apache/spark/sql/api/r/SQLUtils.scala

felixcheung · 2018-12-01T07:05:07Z

R/pkg/R/functions.R

what if as.json.array is TRUE but schema is also set?

If so, the provided schema is wrapped by Array. The test cases are ...

here https://github.com/apache/spark/pull/23184/files#diff-d4011863c8b176830365b2f224a84bf2R1707

sql/core/src/main/scala/org/apache/spark/sql/api/r/SQLUtils.scala

felixcheung · 2018-12-01T07:08:43Z

R/pkg/R/functions.R

we should probably try to pull all the setClassUnion in one place. (to avoid conflict or duplication)

Yup, I agree.. Would you mind if I do this separately? I roughly checked by grep and looks:

./pkg/R/DataFrame.R:setClassUnion("characterOrstructType", c("character", "structType")) ./pkg/R/DataFrame.R:setClassUnion("numericOrcharacter", c("numeric", "character")) ./pkg/R/DataFrame.R:setClassUnion("characterOrColumn", c("character", "Column")) ./pkg/R/DataFrame.R:setClassUnion("numericOrColumn", c("numeric", "Column"))

R/pkg/R/functions.R

SparkQA · 2018-12-01T13:10:51Z

Test build #99547 has finished for PR 23184 at commit c731ad1.

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

HyukjinKwon · 2018-12-20T15:44:04Z

retest this please

HyukjinKwon · 2018-12-20T15:44:23Z

Hey, @felixcheung, mind if I ask to take a look when you're available please?

SparkQA · 2018-12-20T19:43:39Z

Test build #100335 has finished for PR 23184 at commit c731ad1.

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

felixcheung · 2018-12-25T19:46:43Z

R/pkg/R/functions.R

-              jschema <- schema@jc
-            } else if (is.character(schema)) {
+            if (class(schema) == "structType") {
+              schema <- callJMethod(schema$job, "toDDL")


schema$jobj? is there test for this?

I don't quite get this - is it because from_csv doesn't take StructType?

Hm. Weird, this is being tested below from_csv(df$col, structType("a INT")).

Yup, I did this because from_csv with Java map doesn't take StructType ..

spark/sql/core/src/main/scala/org/apache/spark/sql/functions.scala

Line 3790 in 7c7fccf

def from_csv(e: Column, schema: Column, options: java.util.Map[String, String]): Column = {

BTW, I have no idea why it works:

> structType("a int")$job Java ref type org.apache.spark.sql.types.StructType id 6 > structType("a int")$jobj Java ref type org.apache.spark.sql.types.StructType id 8 > structType("a int")$j Java ref type org.apache.spark.sql.types.StructType id 10

looks that's why it passed. Let me fix it anyway.

R has some funky partial/prefix matching

Haha. I haven't been aware of that so far!

felixcheung · 2018-12-25T19:48:56Z

R/pkg/R/functions.R

+            }
+
+            if (is.character(schema)) {
              jschema <- callJStatic("org.apache.spark.sql.functions", "lit", schema)


hmm, why in the case for from_json, if schema is character is structType(schema)$jobj
where for from_csv, is callJStatic("org.apache.spark.sql.functions", "lit", schema)
?

Ah, yea, that looks a bit confusing. It's similar reason. Fortunately, from_json has StructType with Java Map. So we can directly call it from R side.

spark/sql/core/src/main/scala/org/apache/spark/sql/functions.scala

Line 3407 in 7c7fccf

def from_json(e: Column, schema: StructType, options: java.util.Map[String, String]): Column =

SparkQA · 2018-12-28T14:15:45Z

Test build #100496 has finished for PR 23184 at commit 66e6290.

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

felixcheung · 2019-01-01T01:44:35Z

I see - I'm good with this. LGTM. sounds like Scala API can be made more consistent though?

HyukjinKwon · 2019-01-01T01:53:13Z

Yea, Scala side change is already made about from_[csv|json](schema_of_[csv|json]()).

However, about #23184 (comment) and #23184 (comment), yea, the signatures are not consistent ..

One possibility that was considered before is to match it to only use Column across API (vaguely talked with Wenchen in .. somewhere of a PR before), but it's too breaking change..
There's concern that functions.scala file is getting too long so it's been avoided to add overriden versions of such APIs.

I think we should handle this problem for whole APIs later ..

HyukjinKwon · 2019-01-02T00:00:58Z

Merged to master.

Thanks, @felixcheung and @viirya.

…n R API ## What changes were proposed in this pull request? **1. Document `from_csv(..., schema_of_csv(...))` support:** ```R csv <- "Amsterdam,2018" df <- sql(paste0("SELECT '", csv, "' as csv")) head(select(df, from_csv(df$csv, schema_of_csv(csv)))) ``` ``` from_csv(csv) 1 Amsterdam, 2018 ``` **2. Allow `from_json(..., schema_of_json(...))`** Before: ```R df2 <- sql("SELECT named_struct('name', 'Bob') as people") df2 <- mutate(df2, people_json = to_json(df2$people)) head(select(df2, from_json(df2$people_json, schema_of_json(head(df2)$people_json)))) ``` ``` Error in (function (classes, fdef, mtable) : unable to find an inherited method for function ‘from_json’ for signature ‘"Column", "Column"’ ``` After: ```R df2 <- sql("SELECT named_struct('name', 'Bob') as people") df2 <- mutate(df2, people_json = to_json(df2$people)) head(select(df2, from_json(df2$people_json, schema_of_json(head(df2)$people_json)))) ``` ``` from_json(people_json) 1 Bob ``` **3. (While I'm here) Allow `structType` as schema for `from_csv` support to match with `from_json`.** Before: ```R csv <- "Amsterdam,2018" df <- sql(paste0("SELECT '", csv, "' as csv")) head(select(df, from_csv(df$csv, structType("city STRING, year INT")))) ``` ``` Error in (function (classes, fdef, mtable) : unable to find an inherited method for function ‘from_csv’ for signature ‘"Column", "structType"’ ``` After: ```R csv <- "Amsterdam,2018" df <- sql(paste0("SELECT '", csv, "' as csv")) head(select(df, from_csv(df$csv, structType("city STRING, year INT")))) ``` ``` from_csv(csv) 1 Amsterdam, 2018 ``` ## How was this patch tested? Manually tested and unittests were added. Closes apache#23184 from HyukjinKwon/SPARK-26227-1. Authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Hyukjin Kwon <[email protected]>

HyukjinKwon force-pushed the SPARK-26227-1 branch from 193d688 to 8877837 Compare November 30, 2018 03:51

viirya reviewed Nov 30, 2018

View reviewed changes

R/pkg/R/functions.R Outdated Show resolved Hide resolved

sql/core/src/main/scala/org/apache/spark/sql/api/r/SQLUtils.scala Outdated Show resolved Hide resolved

sql/core/src/main/scala/org/apache/spark/sql/api/r/SQLUtils.scala Outdated Show resolved Hide resolved

felixcheung reviewed Dec 1, 2018

View reviewed changes

HyukjinKwon added 2 commits December 1, 2018 16:57

from_[csv|json] should accept schema_of_[csv|json] in R API

fdecb9a

Address comments

c731ad1

HyukjinKwon force-pushed the SPARK-26227-1 branch from 8877837 to c731ad1 Compare December 1, 2018 09:54

felixcheung reviewed Dec 25, 2018

View reviewed changes

Fix typo

66e6290

asfgit closed this in 39a0493 Jan 2, 2019

HyukjinKwon deleted the SPARK-26227-1 branch March 3, 2020 01:20

[SPARK-26227][R] from_[csv|json] should accept schema_of_[csv|json] in R API #23184

[SPARK-26227][R] from_[csv|json] should accept schema_of_[csv|json] in R API #23184

Uh oh!

Conversation

HyukjinKwon commented Nov 30, 2018

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

HyukjinKwon commented Nov 30, 2018

Uh oh!

SparkQA commented Nov 30, 2018

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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!

Uh oh!

SparkQA commented Dec 1, 2018

Uh oh!

HyukjinKwon commented Dec 20, 2018

Uh oh!

HyukjinKwon commented Dec 20, 2018

Uh oh!

SparkQA commented Dec 20, 2018

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!

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!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Dec 28, 2018

Uh oh!

felixcheung commented Jan 1, 2019

Uh oh!

HyukjinKwon commented Jan 1, 2019

Uh oh!

HyukjinKwon commented Jan 2, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants