You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "LuciferYang (via GitHub)" <gi...@apache.org> on 2023/03/08 08:50:18 UTC
[GitHub] [spark] LuciferYang opened a new pull request, #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
LuciferYang opened a new pull request, #40332:
URL: https://github.com/apache/spark/pull/40332
### What changes were proposed in this pull request?
This pr add a new proto message
```
message Parse {
// (Required) Input relation to Parse. The input is expected to have single text column.
Relation input = 1;
// (Required) The expected format of the text.
ParseFormat format = 2;
// Options for the csv/json parser. The map key is case insensitive.
map<string, string> options = 3;
enum ParseFormat {
PARSE_FORMAT_UNSPECIFIED = 0;
PARSE_FORMAT_CSV = 1;
PARSE_FORMAT_JSON = 2;
}
}
```
and implement CSV/JSON parsing functions for Scala client.
### Why are the changes needed?
Add Spark connect jvm client api coverage.
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Pass Github Actions
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng closed pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng closed pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
URL: https://github.com/apache/spark/pull/40332
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1130368098
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/DataFrameReader.scala:
##########
@@ -504,6 +542,18 @@ class DataFrameReader private[sql] (sparkSession: SparkSession) extends Logging
}
}
+ private def parse(ds: Dataset[String], format: ParseFormat): DataFrame = {
+ sparkSession.newDataFrame { builder =>
+ val parseBuilder = builder.getParseBuilder
+ .setInput(ds.plan.getRoot)
+ .setFormat(format)
+ userSpecifiedSchema.foreach(schema => parseBuilder.setSchema(schema.toDDL))
Review Comment:
What do you think about this? @hvanhovell
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40332:
URL: https://github.com/apache/spark/pull/40332#issuecomment-1460106740
not sure whether I’m missing something, but isn’t the schema already provided by users?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129567876
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/PlanGenerationTestSuite.scala:
##########
@@ -257,6 +258,13 @@ class PlanGenerationTestSuite
session.read.json(testDataPath.resolve("people.json").toString)
}
+ test("json from dataset") {
+ session.read
+ .schema(new StructType().add("c1", StringType).add("c2", IntegerType))
+ .option("allowSingleQuotes", "true")
+ .json(session.createDataset(Seq.empty[String])(StringEncoder))
Review Comment:
`session.emptyDataset(StringEncoder)`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129398907
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -644,6 +645,34 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
.collect()
assert(result sameElements expected)
}
+
+ test("json from Dataset[String]") {
+ val session = spark
+ import session.implicits._
+ val expected = Seq(
+ new GenericRowWithSchema(
+ Array(73, "Shandong", "Kong"),
+ new StructType().add("age", LongType).add("city", StringType).add("name", StringType)))
+ val ds = Seq("""{"name":"Kong","age":73,"city":'Shandong'}""").toDS()
+ val result = spark.read.option("allowSingleQuotes", "true").json(ds)
+ checkSameResult(expected, result)
Review Comment:
Probably we should add the user provided schema in the message? Or always discard it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129582810
##########
connector/connect/common/src/main/protobuf/spark/connect/relations.proto:
##########
@@ -793,3 +794,23 @@ message CollectMetrics {
// (Required) The metric sequence.
repeated Expression metrics = 3;
}
+
+message Parse {
+ // (Required) Input relation to Parse. The input is expected to have single text column.
+ Relation input = 1;
+ // (Required) The expected format of the text.
+ ParseFormat format = 2;
+
+ // (Optional) If not set, Spark will infer the schema.
+ //
+ // This schema string should be either DDL-formatted or JSON-formatted.
+ optional string schema = 3;
Review Comment:
https://github.com/apache/spark/pull/40332#discussion_r1129398907 & https://github.com/apache/spark/pull/40332#discussion_r1129404657
and I think the `userSpecifiedSchema` can be different from inferSchema？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129584996
##########
connector/connect/common/src/main/protobuf/spark/connect/relations.proto:
##########
@@ -793,3 +794,23 @@ message CollectMetrics {
// (Required) The metric sequence.
repeated Expression metrics = 3;
}
+
+message Parse {
+ // (Required) Input relation to Parse. The input is expected to have single text column.
+ Relation input = 1;
+ // (Required) The expected format of the text.
+ ParseFormat format = 2;
+
+ // (Optional) If not set, Spark will infer the schema.
+ //
+ // This schema string should be either DDL-formatted or JSON-formatted.
+ optional string schema = 3;
Review Comment:
So if userSpecifiedSchema is set, we should pass it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129410862
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -644,6 +645,34 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
.collect()
assert(result sameElements expected)
}
+
+ test("json from Dataset[String]") {
+ val session = spark
+ import session.implicits._
+ val expected = Seq(
+ new GenericRowWithSchema(
+ Array(73, "Shandong", "Kong"),
+ new StructType().add("age", LongType).add("city", StringType).add("name", StringType)))
+ val ds = Seq("""{"name":"Kong","age":73,"city":'Shandong'}""").toDS()
+ val result = spark.read.option("allowSingleQuotes", "true").json(ds)
+ checkSameResult(expected, result)
Review Comment:
Yes, I think you are right, we should add `schema` to the message if it exists, thanks ~ I will update it later
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129571353
##########
connector/connect/common/src/test/resources/query-tests/explain-results/csv_from_dataset.explain:
##########
@@ -0,0 +1 @@
+LogicalRDD [c1#0, c2#0], false
Review Comment:
Oh this makes me sad. We are we using RDDs here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129396419
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -644,6 +645,34 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
.collect()
assert(result sameElements expected)
}
+
+ test("json from Dataset[String]") {
+ val session = spark
+ import session.implicits._
+ val expected = Seq(
+ new GenericRowWithSchema(
+ Array(73, "Shandong", "Kong"),
+ new StructType().add("age", LongType).add("city", StringType).add("name", StringType)))
+ val ds = Seq("""{"name":"Kong","age":73,"city":'Shandong'}""").toDS()
+ val result = spark.read.option("allowSingleQuotes", "true").json(ds)
+ checkSameResult(expected, result)
Review Comment:
Make sense, you are right
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129569030
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/connect/client/CheckConnectJvmClientCompatibility.scala:
##########
@@ -131,7 +131,6 @@ object CheckConnectJvmClientCompatibility {
// DataFrame Reader & Writer
ProblemFilters.exclude[Problem]("org.apache.spark.sql.DataFrameReader.json"),
Review Comment:
There is no way we can support those, so this is the best we can do.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1130414814
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/DataFrameReader.scala:
##########
@@ -504,6 +542,18 @@ class DataFrameReader private[sql] (sparkSession: SparkSession) extends Logging
}
}
+ private def parse(ds: Dataset[String], format: ParseFormat): DataFrame = {
+ sparkSession.newDataFrame { builder =>
+ val parseBuilder = builder.getParseBuilder
+ .setInput(ds.plan.getRoot)
+ .setFormat(format)
+ userSpecifiedSchema.foreach(schema => parseBuilder.setSchema(schema.toDDL))
Review Comment:
In this scenario, I think `DataType schema` is used and `schema.toDDL` is no longer needed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129391698
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -644,6 +645,34 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
.collect()
assert(result sameElements expected)
}
+
+ test("json from Dataset[String]") {
+ val session = spark
+ import session.implicits._
+ val expected = Seq(
+ new GenericRowWithSchema(
+ Array(73, "Shandong", "Kong"),
+ new StructType().add("age", LongType).add("city", StringType).add("name", StringType)))
+ val ds = Seq("""{"name":"Kong","age":73,"city":'Shandong'}""").toDS()
+ val result = spark.read.option("allowSingleQuotes", "true").json(ds)
+ checkSameResult(expected, result)
Review Comment:
https://github.com/apache/spark/blob/69dd20b5e45c7e3533efbfdc1974f59931c1b781/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L404-L417
From the code of the server side, `userSpecifiedSchema` is an `Option[StructType]` and default is `None`, so we can use it without specifying the`userSpecifiedSchema`? Or is my test case not the correct scenario?
@zhengruifeng
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] hvanhovell commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "hvanhovell (via GitHub)" <gi...@apache.org>.
hvanhovell commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129570585
##########
connector/connect/common/src/main/protobuf/spark/connect/relations.proto:
##########
@@ -793,3 +794,23 @@ message CollectMetrics {
// (Required) The metric sequence.
repeated Expression metrics = 3;
}
+
+message Parse {
+ // (Required) Input relation to Parse. The input is expected to have single text column.
+ Relation input = 1;
+ // (Required) The expected format of the text.
+ ParseFormat format = 2;
+
+ // (Optional) If not set, Spark will infer the schema.
+ //
+ // This schema string should be either DDL-formatted or JSON-formatted.
+ optional string schema = 3;
Review Comment:
We can't use the actual data type?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129673728
##########
connector/connect/common/src/test/resources/query-tests/explain-results/csv_from_dataset.explain:
##########
@@ -0,0 +1 @@
+LogicalRDD [c1#0, c2#0], false
Review Comment:
https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L424-L433
https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L503-L521
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129170162
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/connect/client/CheckConnectJvmClientCompatibility.scala:
##########
@@ -131,7 +131,6 @@ object CheckConnectJvmClientCompatibility {
// DataFrame Reader & Writer
ProblemFilters.exclude[Problem]("org.apache.spark.sql.DataFrameReader.json"),
Review Comment:
still two deprecated `.json` functions no correspondent `client` version , so can't remove `ProblemFilters.exclude[Problem]("org.apache.spark.sql.DataFrameReader.json")`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40332:
URL: https://github.com/apache/spark/pull/40332#issuecomment-1461430958
@LuciferYang thank you for woking on this.
merged into master/branch-3.4
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129574656
##########
connector/connect/server/src/main/scala/org/apache/spark/sql/connect/planner/SparkConnectPlanner.scala:
##########
@@ -710,6 +712,33 @@ class SparkConnectPlanner(val session: SparkSession) {
}
}
+ private def transformParse(rel: proto.Parse): LogicalPlan = {
+ def dataFrameReader = {
+ val localMap = CaseInsensitiveMap[String](rel.getOptionsMap.asScala.toMap)
+ val reader = session.read
+ if (rel.hasSchema && rel.getSchema.nonEmpty) {
+ DataType.parseTypeWithFallback(
+ rel.getSchema,
+ StructType.fromDDL,
+ fallbackParser = DataType.fromJson) match {
+ case s: StructType => reader.schema(s)
+ case other => throw InvalidPlanInput(s"Invalid schema $other")
+ }
+ }
+ localMap.foreach { case (key, value) => reader.option(key, value) }
+ reader
+ }
+ def ds: Dataset[String] = Dataset(session, transformRelation(rel.getInput))(Encoders.STRING)
+
+ rel.getFormat match {
+ case ParseFormat.PARSE_FORMAT_CSV =>
+ dataFrameReader.csv(ds).logicalPlan
+ case ParseFormat.PARSE_FORMAT_JSON =>
+ dataFrameReader.json(ds).logicalPlan
Review Comment:
Am I wrong here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129673728
##########
connector/connect/common/src/test/resources/query-tests/explain-results/csv_from_dataset.explain:
##########
@@ -0,0 +1 @@
+LogicalRDD [c1#0, c2#0], false
Review Comment:
https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L424-L433
https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L503-L521
https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala#L560-L571
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129391698
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -644,6 +645,34 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
.collect()
assert(result sameElements expected)
}
+
+ test("json from Dataset[String]") {
+ val session = spark
+ import session.implicits._
+ val expected = Seq(
+ new GenericRowWithSchema(
+ Array(73, "Shandong", "Kong"),
+ new StructType().add("age", LongType).add("city", StringType).add("name", StringType)))
+ val ds = Seq("""{"name":"Kong","age":73,"city":'Shandong'}""").toDS()
+ val result = spark.read.option("allowSingleQuotes", "true").json(ds)
+ checkSameResult(expected, result)
Review Comment:
https://github.com/apache/spark/blob/69dd20b5e45c7e3533efbfdc1974f59931c1b781/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L404-L417
From the code of the server side, `userSpecifiedSchema` is an `Option[StructType]` and default is None, so we can use it without specifying the`userSpecifiedSchema`? Or is my test case not the correct scenario?
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -644,6 +645,34 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
.collect()
assert(result sameElements expected)
}
+
+ test("json from Dataset[String]") {
+ val session = spark
+ import session.implicits._
+ val expected = Seq(
+ new GenericRowWithSchema(
+ Array(73, "Shandong", "Kong"),
+ new StructType().add("age", LongType).add("city", StringType).add("name", StringType)))
+ val ds = Seq("""{"name":"Kong","age":73,"city":'Shandong'}""").toDS()
+ val result = spark.read.option("allowSingleQuotes", "true").json(ds)
+ checkSameResult(expected, result)
Review Comment:
https://github.com/apache/spark/blob/69dd20b5e45c7e3533efbfdc1974f59931c1b781/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L404-L417
From the code of the server side, `userSpecifiedSchema` is an `Option[StructType]` and default is None, so we can use it without specifying the`userSpecifiedSchema`? Or is my test case not the correct scenario?
@zhengruifeng
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129404657
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -644,6 +645,34 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
.collect()
assert(result sameElements expected)
}
+
+ test("json from Dataset[String]") {
+ val session = spark
+ import session.implicits._
+ val expected = Seq(
+ new GenericRowWithSchema(
+ Array(73, "Shandong", "Kong"),
+ new StructType().add("age", LongType).add("city", StringType).add("name", StringType)))
+ val ds = Seq("""{"name":"Kong","age":73,"city":'Shandong'}""").toDS()
+ val result = spark.read.option("allowSingleQuotes", "true").json(ds)
+ checkSameResult(expected, result)
Review Comment:
Will inferFromDataset trigger an job? If so, I think we’d better skip it if possible
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129582810
##########
connector/connect/common/src/main/protobuf/spark/connect/relations.proto:
##########
@@ -793,3 +794,23 @@ message CollectMetrics {
// (Required) The metric sequence.
repeated Expression metrics = 3;
}
+
+message Parse {
+ // (Required) Input relation to Parse. The input is expected to have single text column.
+ Relation input = 1;
+ // (Required) The expected format of the text.
+ ParseFormat format = 2;
+
+ // (Optional) If not set, Spark will infer the schema.
+ //
+ // This schema string should be either DDL-formatted or JSON-formatted.
+ optional string schema = 3;
Review Comment:
https://github.com/apache/spark/pull/40332#discussion_r1129398907 & https://github.com/apache/spark/pull/40332#discussion_r1129404657
and I think the `userSpecifiedSchema` can be different from `infered schema`？
##########
connector/connect/common/src/test/resources/query-tests/explain-results/csv_from_dataset.explain:
##########
@@ -0,0 +1 @@
+LogicalRDD [c1#0, c2#0], false
Review Comment:
On the server side, the input `csvDataset` and `jsonDataset` are still `LocalRelation`, and the above code path(`sparkSession.internalCreateDataFrame`) is converted them to `RDD`.
##########
connector/connect/common/src/test/resources/query-tests/explain-results/csv_from_dataset.explain:
##########
@@ -0,0 +1 @@
+LogicalRDD [c1#0, c2#0], false
Review Comment:
On the server side, the input `csvDataset` and `jsonDataset` are still `LocalRelation`, and the above code path(`sparkSession.internalCreateDataFrame`) is converted them to `LogicalRDD `.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129682029
##########
connector/connect/common/src/test/resources/query-tests/explain-results/csv_from_dataset.explain:
##########
@@ -0,0 +1 @@
+LogicalRDD [c1#0, c2#0], false
Review Comment:
On the server side, the input `csvDataset` and `jsonDataset` are still `LocalRelation`, and the above code path is converted them to `RDD`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1130412260
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/DataFrameReader.scala:
##########
@@ -504,6 +542,18 @@ class DataFrameReader private[sql] (sparkSession: SparkSession) extends Logging
}
}
+ private def parse(ds: Dataset[String], format: ParseFormat): DataFrame = {
+ sparkSession.newDataFrame { builder =>
+ val parseBuilder = builder.getParseBuilder
+ .setInput(ds.plan.getRoot)
+ .setFormat(format)
+ userSpecifiedSchema.foreach(schema => parseBuilder.setSchema(schema.toDDL))
Review Comment:
[ca6ec7b](https://github.com/apache/spark/pull/40332/commits/ca6ec7b4651b3e9c69bc96240687840179b49ff0) rename `data_type` to `schema` in proto message
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #40332:
URL: https://github.com/apache/spark/pull/40332#issuecomment-1459996898
> why not using `from_json` and `from_csv` to do this?
How to get the schema?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40332:
URL: https://github.com/apache/spark/pull/40332#issuecomment-1459952731
why not using `from_json` and `from_csv` to do this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129391698
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -644,6 +645,34 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
.collect()
assert(result sameElements expected)
}
+
+ test("json from Dataset[String]") {
+ val session = spark
+ import session.implicits._
+ val expected = Seq(
+ new GenericRowWithSchema(
+ Array(73, "Shandong", "Kong"),
+ new StructType().add("age", LongType).add("city", StringType).add("name", StringType)))
+ val ds = Seq("""{"name":"Kong","age":73,"city":'Shandong'}""").toDS()
+ val result = spark.read.option("allowSingleQuotes", "true").json(ds)
+ checkSameResult(expected, result)
Review Comment:
https://github.com/apache/spark/blob/69dd20b5e45c7e3533efbfdc1974f59931c1b781/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L404-L417
From the code of the server side, `userSpecifiedSchema` is an `Option[StructType]` and default is `None`, so I think we can use it without specifying the`userSpecifiedSchema` for this function? Or is my test case not the correct scenario?
@zhengruifeng
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129396701
##########
connector/connect/client/jvm/src/test/scala/org/apache/spark/sql/ClientE2ETestSuite.scala:
##########
@@ -644,6 +645,34 @@ class ClientE2ETestSuite extends RemoteSparkSession with SQLHelper {
.collect()
assert(result sameElements expected)
}
+
+ test("json from Dataset[String]") {
+ val session = spark
+ import session.implicits._
+ val expected = Seq(
+ new GenericRowWithSchema(
+ Array(73, "Shandong", "Kong"),
+ new StructType().add("age", LongType).add("city", StringType).add("name", StringType)))
+ val ds = Seq("""{"name":"Kong","age":73,"city":'Shandong'}""").toDS()
+ val result = spark.read.option("allowSingleQuotes", "true").json(ds)
+ checkSameResult(expected, result)
Review Comment:
Thanks ~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #40332:
URL: https://github.com/apache/spark/pull/40332#issuecomment-1461183365
> probably not related to this PR:
>
> https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L63-L76
>
> ```
> def schema(schemaString: String): DataFrameReader = {
> schema(StructType.fromDDL(schemaString))
> }
> ```
>
> when the user provide a DDL string, it invoke the parser. Here I think we should keep both StructType and DDL string, and pass them to the server side.
message `Read` seems also need to consider this？I think we can further discuss this problem in a separate jira?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1130347402
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/DataFrameReader.scala:
##########
@@ -504,6 +542,18 @@ class DataFrameReader private[sql] (sparkSession: SparkSession) extends Logging
}
}
+ private def parse(ds: Dataset[String], format: ParseFormat): DataFrame = {
+ sparkSession.newDataFrame { builder =>
+ val parseBuilder = builder.getParseBuilder
+ .setInput(ds.plan.getRoot)
+ .setFormat(format)
+ userSpecifiedSchema.foreach(schema => parseBuilder.setSchema(schema.toDDL))
Review Comment:
as to this PR itself, I think we should probably use `DataType schema` in the proto message, `schema.toDDL` always discards the metadata.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129582810
##########
connector/connect/common/src/main/protobuf/spark/connect/relations.proto:
##########
@@ -793,3 +794,23 @@ message CollectMetrics {
// (Required) The metric sequence.
repeated Expression metrics = 3;
}
+
+message Parse {
+ // (Required) Input relation to Parse. The input is expected to have single text column.
+ Relation input = 1;
+ // (Required) The expected format of the text.
+ ParseFormat format = 2;
+
+ // (Optional) If not set, Spark will infer the schema.
+ //
+ // This schema string should be either DDL-formatted or JSON-formatted.
+ optional string schema = 3;
Review Comment:
https://github.com/apache/spark/pull/40332#discussion_r1129398907 & https://github.com/apache/spark/pull/40332#discussion_r1129404657
and I think the `userSpecifiedSchema` be different from inferSchema？
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on PR #40332:
URL: https://github.com/apache/spark/pull/40332#issuecomment-1461435665
Thanks @zhengruifeng @hvanhovell @HyukjinKwon ~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1129673728
##########
connector/connect/common/src/test/resources/query-tests/explain-results/csv_from_dataset.explain:
##########
@@ -0,0 +1 @@
+LogicalRDD [c1#0, c2#0], false
Review Comment:
https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L424-L433
https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/sql/core/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L503-L521
https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala#L357-L365
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] LuciferYang commented on a diff in pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "LuciferYang (via GitHub)" <gi...@apache.org>.
LuciferYang commented on code in PR #40332:
URL: https://github.com/apache/spark/pull/40332#discussion_r1130391567
##########
connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/DataFrameReader.scala:
##########
@@ -504,6 +542,18 @@ class DataFrameReader private[sql] (sparkSession: SparkSession) extends Logging
}
}
+ private def parse(ds: Dataset[String], format: ParseFormat): DataFrame = {
+ sparkSession.newDataFrame { builder =>
+ val parseBuilder = builder.getParseBuilder
+ .setInput(ds.plan.getRoot)
+ .setFormat(format)
+ userSpecifiedSchema.foreach(schema => parseBuilder.setSchema(schema.toDDL))
Review Comment:
[20f1722](https://github.com/apache/spark/pull/40332/commits/20f1722c4cc3331e0c8247f4d3cae419a3e81870) change to pass a DataType @zhengruifeng
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] zhengruifeng commented on pull request #40332: [SPARK-42690][CONNECT] Implement CSV/JSON parsing functions for Scala client
Posted by "zhengruifeng (via GitHub)" <gi...@apache.org>.
zhengruifeng commented on PR #40332:
URL: https://github.com/apache/spark/pull/40332#issuecomment-1461175796
probably not related to this PR:
https://github.com/apache/spark/blob/39a55121888d2543a6056be65e0c74126a9d3bdf/connector/connect/client/jvm/src/main/scala/org/apache/spark/sql/DataFrameReader.scala#L63-L76
```
def schema(schemaString: String): DataFrameReader = {
schema(StructType.fromDDL(schemaString))
}
```
when the user provide a DDL string, it invoke the parser. Here I think we should keep both StructType and DDL string, and pass them to the server side.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org