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 2022/05/02 11:48:26 UTC
[GitHub] [spark] ravwojdyla opened a new pull request, #36430: [WIP][SPARK-38904] Select by schema
ravwojdyla opened a new pull request, #36430:
URL: https://github.com/apache/spark/pull/36430
Almost copy pasting from https://issues.apache.org/jira/browse/SPARK-38904:
This PR is related to https://stackoverflow.com/questions/71610435. Let's assume I have a pyspark DataFrame with certain schema, and I would like to select/overwrite that schema with a new schema that I *know* is compatible, I could do:
```python
df: DataFrame
new_schema = ...
df.rdd.toDF(schema=new_schema)
```
Unfortunately this triggers computation as described in https://stackoverflow.com/questions/37088484/whats-the-performance-impact-of-converting-between-dataframe-rdd-and-back/37090151#37090151.
Note:
* the schema can be arbitrarily complicated (nested etc)
* new schema includes updates to description, nullability and additional metadata
See POC of workaround/util in https://github.com/ravwojdyla/spark-schema-utils
Also posted in https://lists.apache.org/thread/5ds0f7chzp1s3h10tvjm3r96g769rvpj
### What changes were proposed in this pull request?
* add `DataFrame.select(schema)`
### Why are the changes needed?
* add `DataFrame.select(schema)`
### Does this PR introduce _any_ user-facing change?
* yes `DataFrame.select(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] HyukjinKwon commented on pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1173146833
> Interesting, so you don't expect collaboration? I've marked this PR as WIP, because I wasn't married to this specific implementation.
How did you reach to that conclusion? I was explaining what happened this and that PR. Collaboration is great but I don't expect contributors or committers know what's going on in every single PR.
I myself didn't find the time to take a closer look of your PR but another contributor spent more time to propose a complete version.
Do you expect that proposal to be rejected and yours have to be reviewed?
--
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] ravwojdyla commented on a diff in pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
ravwojdyla commented on code in PR #36430:
URL: https://github.com/apache/spark/pull/36430#discussion_r863329395
##########
sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -1593,6 +1593,35 @@ class Dataset[T] private[sql](
@scala.annotation.varargs
def select(col: String, cols: String*): DataFrame = select((col +: cols).map(Column(_)) : _*)
+ /**
+ * Selects a set of columns via schema object.
+ */
+ def select(schema: StructType): DataFrame = {
+ val attrs = logicalPlan.output
+ val attrs_map = attrs.map { a => (a.name, a) }.toMap
+ val new_attrs = AttributeMap(schema.map { f =>
Review Comment:
So this current code swaps the `dataType` without checking if it's safe. Do you or @jiangxb1987 have any tips how to safely handle nested data 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] ravwojdyla commented on pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
ravwojdyla commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1173060645
@HyukjinKwon thanks for the response.
> This and https://github.com/apache/spark/pull/37011 cases are a bit different because the implementation and approach are pretty much different. I usually put the author of the first PR as a co-author for the sake of respecting the efforts made but technically the authorship of the fix has to be to the one who proposed the different approach IMO.
Interesting, so you don't expect collaboration? I've marked this PR as WIP, because I wasn't married to this specific implementation. And it was the collaboration that I was hoping for when asking for guidance in https://github.com/apache/spark/pull/36430#discussion_r863267242 (comment above), such that we can find a solution that works best for me, community and Databricks.
At the end of the day I'm excited to see this functionality in Spark, and I'm excited for #37011, will give it a try and provide comments in the other PR if I have any. I'm closing 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] ravwojdyla closed pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
ravwojdyla closed pull request #36430: [WIP][SPARK-38904] Select by schema
URL: https://github.com/apache/spark/pull/36430
--
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] ravwojdyla commented on a diff in pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
ravwojdyla commented on code in PR #36430:
URL: https://github.com/apache/spark/pull/36430#discussion_r887126464
##########
sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -1593,6 +1593,35 @@ class Dataset[T] private[sql](
@scala.annotation.varargs
def select(col: String, cols: String*): DataFrame = select((col +: cols).map(Column(_)) : _*)
+ /**
+ * Selects a set of columns via schema object.
+ */
+ def select(schema: StructType): DataFrame = {
+ val attrs = logicalPlan.output
+ val attrs_map = attrs.map { a => (a.name, a) }.toMap
+ val new_attrs = AttributeMap(schema.map { f =>
Review Comment:
@HyukjinKwon @jiangxb1987 one thing we could do is to validate that the schema we are selecting/swapping by is compatible with the current schema (including nested columns). Would this be an acceptable solution for you?
--
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 pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1173030851
Seems like https://github.com/apache/spark/pull/37011 will supersede this PR
--
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] ravwojdyla commented on pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
ravwojdyla commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1173052953
> Seems like https://github.com/apache/spark/pull/37011 will supersede this PR
@HyukjinKwon that's great, thank you! First of all I appreciate the work that went into #37011 and I'm excited to use. I would like to ask for future reference, what's the actual process for PRs/changes/collaboration from the community? This is the 2nd time this is happening where I submit a PR, there's some response and then someone from Databricks submits their own (arguable better) version. The other PR was https://github.com/apache/spark/pull/34863. FYI @cloud-fan @zero323
--
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] ravwojdyla commented on pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
ravwojdyla commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1114760687
👋 @HyukjinKwon I'm a little blocked on what's the best way to enforce compatibility of nested columns, do you have any hits please? Also any other comments about the current WIP? Would appreciate your input.
--
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 #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1114766715
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] ravwojdyla commented on a diff in pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
ravwojdyla commented on code in PR #36430:
URL: https://github.com/apache/spark/pull/36430#discussion_r867931583
##########
sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -1593,6 +1593,35 @@ class Dataset[T] private[sql](
@scala.annotation.varargs
def select(col: String, cols: String*): DataFrame = select((col +: cols).map(Column(_)) : _*)
+ /**
+ * Selects a set of columns via schema object.
+ */
+ def select(schema: StructType): DataFrame = {
+ val attrs = logicalPlan.output
+ val attrs_map = attrs.map { a => (a.name, a) }.toMap
+ val new_attrs = AttributeMap(schema.map { f =>
Review Comment:
👋 @HyukjinKwon @jiangxb1987 would appreciate your input please ^
--
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] ravwojdyla commented on a diff in pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
ravwojdyla commented on code in PR #36430:
URL: https://github.com/apache/spark/pull/36430#discussion_r884187298
##########
sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -1593,6 +1593,35 @@ class Dataset[T] private[sql](
@scala.annotation.varargs
def select(col: String, cols: String*): DataFrame = select((col +: cols).map(Column(_)) : _*)
+ /**
+ * Selects a set of columns via schema object.
+ */
+ def select(schema: StructType): DataFrame = {
+ val attrs = logicalPlan.output
+ val attrs_map = attrs.map { a => (a.name, a) }.toMap
+ val new_attrs = AttributeMap(schema.map { f =>
Review Comment:
@HyukjinKwon @jiangxb1987 would appreciate your input please ^
--
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] ravwojdyla commented on pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
ravwojdyla commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1173149029
> Do you expect that proposal to be rejected and yours has to be reviewed?
@HyukjinKwon ofc not, from previous comment:
> At the end of the day I'm excited to see this functionality in Spark, and I'm excited for https://github.com/apache/spark/pull/37011, will give it a try and provide comments in the other PR if I have any. I'm closing 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] HyukjinKwon commented on pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1173057743
In https://github.com/apache/spark/pull/34863 case, I slept through my fingers. I think yours had to be merged. I am sorry that was my bad.
This and https://github.com/apache/spark/pull/37011 cases are a bit different because the implementation and approach are pretty much different. I usually put the author of the first PR as a co-author for the sake of respecting the efforts made but technically the authorship of the fix has to be to the one who proposed the different approach IMO.
This isn't specific to a specific company, see also https://github.com/apache/spark/pull/36917.
--
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] cloud-fan commented on pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1173669357
Oh, I didn't know this PR before submitting https://github.com/apache/spark/pull/37011 . However, I think this PR is still a draft and https://github.com/apache/spark/pull/37011 is much more ready-to-merge.
Conflicting PRs are inevitable as no one can know all the ongoing work. Usually, the first PR wins, but if the approaches are very different, the better version wins.
--
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 diff in pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on code in PR #36430:
URL: https://github.com/apache/spark/pull/36430#discussion_r863267242
##########
sql/core/src/main/scala/org/apache/spark/sql/Dataset.scala:
##########
@@ -1593,6 +1593,35 @@ class Dataset[T] private[sql](
@scala.annotation.varargs
def select(col: String, cols: String*): DataFrame = select((col +: cols).map(Column(_)) : _*)
+ /**
+ * Selects a set of columns via schema object.
+ */
+ def select(schema: StructType): DataFrame = {
+ val attrs = logicalPlan.output
+ val attrs_map = attrs.map { a => (a.name, a) }.toMap
+ val new_attrs = AttributeMap(schema.map { f =>
Review Comment:
We will have to make it working with nested fields I think ..
--
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 pull request #36430: [WIP][SPARK-38904] Select by schema
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on PR #36430:
URL: https://github.com/apache/spark/pull/36430#issuecomment-1115478312
@jiangxb1987 who were interested in 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