You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "huaxingao (via GitHub)" <gi...@apache.org> on 2023/04/21 01:43:11 UTC
[GitHub] [spark] huaxingao opened a new pull request, #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
huaxingao opened a new pull request, #40889:
URL: https://github.com/apache/spark/pull/40889
### What changes were proposed in this pull request?
backporting https://github.com/apache/spark/pull/39152 to 3.3
### Why are the changes needed?
bug fixing
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
UT
--
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] huaxingao commented on pull request #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on PR #40889:
URL: https://github.com/apache/spark/pull/40889#issuecomment-1517948030
Merged to branch-3.3. Thank you all for reviewing!
--
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] viirya commented on a diff in pull request #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #40889:
URL: https://github.com/apache/spark/pull/40889#discussion_r1173259726
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -925,7 +925,7 @@ class Analyzer(override val catalogManager: CatalogManager)
if (metaCols.isEmpty) {
node
} else {
- val newNode = addMetadataCol(node)
+ val newNode = node.mapChildren(addMetadataCol(_, metaCols.map(_.exprId).toSet))
Review Comment:
Oh, there was follow up.
--
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] huaxingao commented on a diff in pull request #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #40889:
URL: https://github.com/apache/spark/pull/40889#discussion_r1173243674
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -925,7 +925,7 @@ class Analyzer(override val catalogManager: CatalogManager)
if (metaCols.isEmpty) {
node
} else {
- val newNode = addMetadataCol(node)
+ val newNode = node.mapChildren(addMetadataCol(_, metaCols.map(_.exprId).toSet))
Review Comment:
Yes, master branch has the same code.
--
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] viirya commented on a diff in pull request #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on code in PR #40889:
URL: https://github.com/apache/spark/pull/40889#discussion_r1173227832
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -925,7 +925,7 @@ class Analyzer(override val catalogManager: CatalogManager)
if (metaCols.isEmpty) {
node
} else {
- val newNode = addMetadataCol(node)
+ val newNode = node.mapChildren(addMetadataCol(_, metaCols.map(_.exprId).toSet))
Review Comment:
Why this only does `addMetadataCol` on children?
--
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] huaxingao closed pull request #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao closed pull request #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
URL: https://github.com/apache/spark/pull/40889
--
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] huaxingao commented on pull request #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on PR #40889:
URL: https://github.com/apache/spark/pull/40889#issuecomment-1517140043
cc @cloud-fan @viirya @RussellSpitzer
--
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 a diff in pull request #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #40889:
URL: https://github.com/apache/spark/pull/40889#discussion_r1173242572
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -925,7 +925,7 @@ class Analyzer(override val catalogManager: CatalogManager)
if (metaCols.isEmpty) {
node
} else {
- val newNode = addMetadataCol(node)
+ val newNode = node.mapChildren(addMetadataCol(_, metaCols.map(_.exprId).toSet))
Review Comment:
This looks correct to me because we need to propagate metadata cols to this node and need to add metadata cols in its children. Does master branch have the same code 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] huaxingao commented on a diff in pull request #40889: [SPARK-41660][SQL][3.3] Only propagate metadata columns if they are used
Posted by "huaxingao (via GitHub)" <gi...@apache.org>.
huaxingao commented on code in PR #40889:
URL: https://github.com/apache/spark/pull/40889#discussion_r1173242176
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala:
##########
@@ -925,7 +925,7 @@ class Analyzer(override val catalogManager: CatalogManager)
if (metaCols.isEmpty) {
node
} else {
- val newNode = addMetadataCol(node)
+ val newNode = node.mapChildren(addMetadataCol(_, metaCols.map(_.exprId).toSet))
Review Comment:
The change is from this fix https://github.com/apache/spark/pull/39895
--
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