You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/10/21 17:28:20 UTC
[GitHub] [spark] risinga opened a new pull request #34359: [SPARK-36986] - Improving external schema management flexibility on DataSet and StructType
risinga opened a new pull request #34359:
URL: https://github.com/apache/spark/pull/34359
### What changes were proposed in this pull request?
These are the following proposed improvements:
1 - ability to retrieve from StructType, the field's name and schema in one single call, requesting to return a tupple by index.
2 - Allowing for a dataset to be created from a schema, and passing the corresponding internal rows which the internal types map with the schema already defined externally.
### Why are the changes needed?
Explanations provided for the respective changes mentioned earlier
1- Avoids two client calls/loops to obtain consolidated field info, when looping through a schema and updating field values on a genericRow data type.
2 - This allows to create Spark fields based on any data structure, without depending on Spark's internal conversions (in particular for Json parsing), and improves performance by skipping the CatalystConverts job of converting native Java types into Spark types.
### Does this PR introduce _any_ user-facing change?
This PR augments the current DataSet API and the StructType object manipulation.
### How was this patch tested?
Unit tests were added (included in the PR), and bench tests were done locally to validate the performance of the new dataset API call.
--
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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r741859708
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
@HyukjinKwon - should I go ahead and remove this method for the time being, so we can move forward with this PR? Or is there an opportunity to improve elsewhere? Please let me know your thoughts.
Happy to brainstorm further!
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
Hello @HyukjinKwon, I'm trying to connect the dots, and map your suggestion with the set goals.
I agree, internal rows are by principle internal and generated off the conversion between the external RDD types and the internal types. The main intention with this change, as mentioned on the PR description:
1 - allow additional flexibility when creating a typed DataSet, and corresponding rows outside Spark. The first change on StructType helps, because avoids unnecessary loops when building the rows.
2 - Improve overall performance with the external building of Spark rows. By passing the InternalRows in the dataset creation, we're bypassing the conversion to Catalyst rows and improving the general performance of the driver.
So, assuming this not a path we want to pursuit - creating internalRows outside - we still need to create a dataframe based on the exposed Row type (at least that is how we need to do it in our project).
For that we can just used the existing method createDataFrame(rowRDD: RDD[Row], schema: StructType): DataFrame
The above suggestion is assuming there is already a dataframe from which we would retrieve the Spark plan with the internal rows. What is exactly the intention with creating a DF from another DF if there isn't any change on data and schema? Apologies if it's an obvious answer, and I didn't get there yet. I'm a Spark contributor newbie :)
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
@HyukjinKwon - should I go ahead and remove this method for the time being, so we can move forward with this PR? Or is there an opportunity to improve elsewhere? Please let me know your thoughts.
Happy to brainstorm further!
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
Hello @HyukjinKwon, I'm trying to connect the dots, and map your suggestion with the set goals.
I agree, internal rows are by principle internal and generated off the conversion between the external RDD types and the internal types. The main intention with this change, as mentioned on the PR description:
1 - allow additional flexibility when creating a typed DataSet, and corresponding rows outside Spark. The first change on StructType helps, because avoids unnecessary loops when building the rows.
2 - Improve overall performance with the external building of Spark rows. By passing the InternalRows in the dataset creation, we're bypassing the conversion to Catalyst rows and improving the general performance of the driver.
So, assuming this not a path we want to pursuit - creating internalRows outside - we still need to create a dataframe based on the exposed Row type (at least that is how we need to do it in our project).
For that we can just used the existing method createDataFrame(rowRDD: RDD[Row], schema: StructType): DataFrame
The above suggestion is assuming there is already a dataframe from which we would retrieve the Spark plan with the internal rows. What is exactly the intention with creating a DF from another DF if there isn't any change on data and schema? Apologies if it's an obvious answer, and I didn't get there yet. I'm a Spark contributor newbie :)
--
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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r746418706
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
@HyukjinKwon, if I don't hear back from you during the next couple of days, I'm going to remove this code, and just keep other change, so we can move forward this PR. 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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r741859708
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
@HyukjinKwon - should I go ahead and remove this method for the time being, so we can move forward with this PR? Or is there an opportunity to improve elsewhere? Please let me know your thoughts.
Happy to brainstorm further!
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
Hello @HyukjinKwon, I'm trying to connect the dots, and map your suggestion with the set goals.
I agree, internal rows are by principle internal and generated off the conversion between the external RDD types and the internal types. The main intention with this change, as mentioned on the PR description:
1 - allow additional flexibility when creating a typed DataSet, and corresponding rows outside Spark. The first change on StructType helps, because avoids unnecessary loops when building the rows.
2 - Improve overall performance with the external building of Spark rows. By passing the InternalRows in the dataset creation, we're bypassing the conversion to Catalyst rows and improving the general performance of the driver.
So, assuming this not a path we want to pursuit - creating internalRows outside - we still need to create a dataframe based on the exposed Row type (at least that is how we need to do it in our project).
For that we can just used the existing method createDataFrame(rowRDD: RDD[Row], schema: StructType): DataFrame
The above suggestion is assuming there is already a dataframe from which we would retrieve the Spark plan with the internal rows. What is exactly the intention with creating a DF from another DF if there isn't any change on data and schema? Apologies if it's an obvious answer, and I didn't get there yet. I'm a Spark contributor newbie :)
--
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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r760404271
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
@HyukjinKwon I've sent an email to dev@spark.apache.org, as well as dev-subscribe@spark.apache.org and I've got no response. I can find the subject on the mailing lists http://mail-archives.apache.org/mod_mbox/spark-dev/202111.mbox/browser, neither I'm receiving any emails from them. Could you please point me to what I could be doing wrong?
--
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] HyukjinKwon commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r734908797
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
My concern is that `InternalRow` is not supposed to be exposed to end users.
Maybe we should expose `Dataset.ofRows` as a developer API, and do it like:
```scala
Dataset.ofRows(
spark,
org.apache.spark.sql.execution.LogicalRDD(
df.queryExecution.executedPlan.output, df.queryExecution.toRdd)(spark))
```
--
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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r751104430
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
Thanks for coming back. I've sent an email to the dev list. Could you please review it and add any additional comments to this new thread? 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] HyukjinKwon commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r747132197
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
I was thinking about making it as a developer API, and users would be able to do:
```scala
val rdd: RDD[InternalRow] = ...
val attributtes = schema.fields.map(f => AttributeReference(f.name, f.dataType, f.nullable, f.metadata)())
Dataset.ofRows(spark, org.apache.spark.sql.execution.LogicalRDD(attributtes, rdd)(spark))
```
But I am not sure if this is the best approach. Probably we should better raise this discussion into Spark dev mailing (dev@spark.apache.org) list, and discuss further.
--
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] HyukjinKwon commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r747132197
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
I was thinking about making it as a developer API, and users would be able to do:
```scala
val rdd: RDD[InternalRow] = ...
val attributtes = schema.fields.map(f => AttributeReference(f.name, f.dataType, f.nullable, f.metadata)())
Dataset.ofRows(spark, LogicalRDD(attributtes, rdd)(spark))
```
But I am not sure if this is the best approach. Probably we should better raise this discussion into Spark dev mailing (dev@spark.apache.org) list, and discuss further.
--
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] AmplabJenkins commented on pull request #34359: [SPARK-36986] - Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #34359:
URL: https://github.com/apache/spark/pull/34359#issuecomment-948861201
Can one of the admins verify this patch?
--
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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r740386718
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
Hello @HyukjinKwon, I'm trying to connect the dots, and map your suggestion with the set goals.
I agree, internal rows are by principle internal and generated off the conversion between the external RDD types and the internal types. The main intention with this change, as mentioned on the PR description:
1 - allow additional flexibility when creating a typed DataSet, and corresponding rows outside Spark. The first change on StructType helps, because avoids unnecessary loops when building the rows.
2 - Improve overall performance with the external building of Spark rows. By passing the InternalRows in the dataset creation, we're bypassing the conversion to Catalyst rows and improvement the general performance of the driver.
So, assuming this not a path we want to pursuit - creating internalRows outside - we still need to create a dataframe based on the exposed Row type (at least that is how we need to do it in our project).
For that we can just used the existing method createDataFrame(rowRDD: RDD[Row], schema: StructType): DataFrame
The above suggestion is assuming there is already a dataframe from which we would retrieve the Spark plan with the internal rows. What is exactly the intention with creating a DF from another DF if there isn't any change on data and schema? Apologies if it's an obvious answer, and I didn't get there yet. I'm a Spark contributor newbie :)
--
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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r740386718
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
Hello @HyukjinKwon, I'm trying to connect the dots, and map your suggestion with the set goals.
I agree, internal rows are by principle internal and generated off the conversion between the external RDD types and the internal types. The main intention with this change, as mentioned on the PR description:
1 - allow additional flexibility when creating a typed DataSet, and corresponding rows outside Spark. The first change on StructType helps, because avoids unnecessary loops when building the rows.
2 - Improve overall performance with the external building of Spark rows. By passing the InternalRows in the dataset creation, we're bypassing the conversion to Catalyst rows and improvement the general performance of the driver.
So, assuming this not a path we want to pursuit - creating internalRows outside - we still need to create a dataframe based on the exposed Row type (at least that is how we need to do it in our project).
For that we can just used the existing method createDataFrame(rowRDD: RDD[Row], schema: StructType): DataFrame
The above suggestion is assuming there is already a dataframe from which we would retrieve the Spark plan with the internal rows. What is exactly the intention with creating a DF from another DF if there isn't any change on data and schema? Apologies if it's an obvious answer, and I didn't get there yet. I'm a Spark contributor newbie :)
--
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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r741859708
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
@HyukjinKwon - should I go ahead and remove this method for the time being, so we can move forward with this PR? Or is there an opportunity to improve elsewhere? Please let me know your thoughts.
Happy to brainstorm further!
--
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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r740386718
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
Hello @HyukjinKwon, I'm trying to connect the dots, and map your suggestion with the set goals.
I agree, internal rows are by principle internal and generated off the conversion between the external RDD types and the internal types. The main intention with this change, as mentioned on the PR description:
1 - allow additional flexibility when creating a typed DataSet, and corresponding rows outside Spark. The first change on StructType helps, because avoids unnecessary loops when building the rows.
2 - Improve overall performance with the external building of Spark rows. By passing the InternalRows in the dataset creation, we're bypassing the conversion to Catalyst rows and improving the general performance of the driver.
So, assuming this not a path we want to pursuit - creating internalRows outside - we still need to create a dataframe based on the exposed Row type (at least that is how we need to do it in our project).
For that we can just used the existing method createDataFrame(rowRDD: RDD[Row], schema: StructType): DataFrame
The above suggestion is assuming there is already a dataframe from which we would retrieve the Spark plan with the internal rows. What is exactly the intention with creating a DF from another DF if there isn't any change on data and schema? Apologies if it's an obvious answer, and I didn't get there yet. I'm a Spark contributor newbie :)
--
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] risinga commented on a change in pull request #34359: [SPARK-36986][SQL] Improving external schema management flexibility on DataSet and StructType
Posted by GitBox <gi...@apache.org>.
risinga commented on a change in pull request #34359:
URL: https://github.com/apache/spark/pull/34359#discussion_r761822675
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala
##########
@@ -511,6 +511,20 @@ class SparkSession private(
createDataset(data.asScala.toSeq)
}
+ /**
+ * Creates a [[Dataset]] from an RDD of spark.sql.catalyst.InternalRow. This method allows
+ * the caller to create externally the InternalRow set, as we as define the schema externally.
+ *
+ * @since 3.3.0
+ */
+ def createDataset(data: RDD[InternalRow], schema: StructType): DataFrame = {
Review comment:
@AmplabJenkins anyway you could help me to make progress?
--
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