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/01/21 10:44:30 UTC
[GitHub] [spark] kevinwallimann opened a new pull request #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
kevinwallimann opened a new pull request #35270:
URL: https://github.com/apache/spark/pull/35270
### What changes were proposed in this pull request?
The metadata of a `GetStructField` expression is propagated in the `Alias` expression.
### Why are the changes needed?
Currently, in a dataframe with nested structs, when selecting an inner struct, the metadata of that inner struct is lost. For example, suppose
`df.schema.head.dataType.head.metadata`
returns a non-empty Metadata object, then
`df.select('Field0.SubField0').schema.head.metadata`
returns an empty Metadata object
The following snippet demonstrates the issue
```
import org.apache.spark.sql.Row
import org.apache.spark.sql.types.{LongType, MetadataBuilder, StructField, StructType}
val metadataAbc = new MetadataBuilder().putString("my-metadata", "abc").build()
val metadataXyz = new MetadataBuilder().putString("my-metadata", "xyz").build()
val schema = StructType(Seq(
StructField("abc",
StructType(Seq(
StructField("xyz", LongType, nullable = true, metadataXyz)
)), metadata = metadataAbc)))
import scala.collection.JavaConverters._
val data = Seq(Row(Row(1L))).asJava
val df = spark.createDataFrame(data, schema)
println(df.select("abc").schema.head.metadata) // OK, metadata is {"my-metadata":"abc"}
println(df.select("abc.xyz").schema.head.metadata) // NOT OK, metadata is {}, expected {"my-metadata","xyz"}
```
The issue can be reproduced in versions 3.2.0, 3.1.2 and 2.4.8
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
Added a new test
--
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 change in pull request #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #35270:
URL: https://github.com/apache/spark/pull/35270#discussion_r790205037
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala
##########
@@ -172,7 +172,7 @@ case class Alias(child: Expression, name: String)(
val builder = new MetadataBuilder().withMetadata(named.metadata)
nonInheritableMetadataKeys.foreach(builder.remove)
builder.build()
-
+ case structField: GetStructField => structField.metadata
Review comment:
Don't we need to remove keys in `nonInheritableMetadataKeys`?
--
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] Zejnilovic commented on pull request #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
Zejnilovic commented on pull request #35270:
URL: https://github.com/apache/spark/pull/35270#issuecomment-1059166669
Hello, @viirya any news on this review?
--
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] kevinwallimann commented on a change in pull request #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
kevinwallimann commented on a change in pull request #35270:
URL: https://github.com/apache/spark/pull/35270#discussion_r790889962
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala
##########
@@ -137,6 +137,20 @@ class ColumnExpressionSuite extends QueryTest with SharedSparkSession {
assert(newCol.expr.asInstanceOf[NamedExpression].metadata.getString("key") === "value")
}
+ test("as propagates metadata from nested column") {
Review comment:
Yes, done.
--
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 closed pull request #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #35270:
URL: https://github.com/apache/spark/pull/35270
--
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] kevinwallimann commented on a change in pull request #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
kevinwallimann commented on a change in pull request #35270:
URL: https://github.com/apache/spark/pull/35270#discussion_r790890482
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/namedExpressions.scala
##########
@@ -172,7 +172,7 @@ case class Alias(child: Expression, name: String)(
val builder = new MetadataBuilder().withMetadata(named.metadata)
nonInheritableMetadataKeys.foreach(builder.remove)
builder.build()
-
+ case structField: GetStructField => structField.metadata
Review comment:
Yes, you're right. I fixed it and added an additional test.
--
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 change in pull request #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
viirya commented on a change in pull request #35270:
URL: https://github.com/apache/spark/pull/35270#discussion_r790204948
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/ColumnExpressionSuite.scala
##########
@@ -137,6 +137,20 @@ class ColumnExpressionSuite extends QueryTest with SharedSparkSession {
assert(newCol.expr.asInstanceOf[NamedExpression].metadata.getString("key") === "value")
}
+ test("as propagates metadata from nested column") {
Review comment:
Can you add jira number as test name prefix? I.e. `SPARK-34805: ...`
--
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 #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #35270:
URL: https://github.com/apache/spark/pull/35270#issuecomment-1018989286
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] HyukjinKwon commented on pull request #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #35270:
URL: https://github.com/apache/spark/pull/35270#issuecomment-1019388070
cc @viirya FYI
--
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 #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #35270:
URL: https://github.com/apache/spark/pull/35270#issuecomment-1074124094
thanks, merging to master/3.3!
--
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] kevinwallimann commented on pull request #35270: [SPARK-34805][SQL] Propagate metadata from nested columns in Alias
Posted by GitBox <gi...@apache.org>.
kevinwallimann commented on pull request #35270:
URL: https://github.com/apache/spark/pull/35270#issuecomment-1039077591
Hi @viirya Could you please review my changes again?
--
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