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 2020/04/27 23:10:15 UTC
[GitHub] [spark] dongjoon-hyun opened a new pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
dongjoon-hyun opened a new pull request #28377:
URL: https://github.com/apache/spark/pull/28377
Credit to @LiangchangZ and @xuanyuanking , this PR reuses the UT as well as integrate test in #24457. Thanks Liangchang for your solid work.
### What changes were proposed in this pull request?
Make metadata propagatable between Aliases.
### Why are the changes needed?
In Structured Streaming, we added an Alias for TimeWindow by default.
https://github.com/apache/spark/blob/590b9a0132b68d9523e663997def957b2e46dfb1/sql/core/src/main/scala/org/apache/spark/sql/functions.scala#L3272-L3273
For some cases like stream join with watermark and window, users need to add an alias for convenience(we also added one in StreamingJoinSuite). The current metadata handling logic for `as` will lose the watermark metadata
https://github.com/apache/spark/blob/590b9a0132b68d9523e663997def957b2e46dfb1/sql/core/src/main/scala/org/apache/spark/sql/Column.scala#L1049-L1054
and finally cause the AnalysisException:
```
Stream-stream outer join between two streaming DataFrame/Datasets is not supported without a watermark in the join keys, or a watermark on the nullable side and an appropriate range condition
```
### Does this PR introduce any user-facing change?
Bugfix for an alias on time window with watermark.
### How was this patch tested?
New UTs added. One for the functionality and one for explaining the common scenario.
----------------------------------------------------------------
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.
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] dongjoon-hyun commented on a change in pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28377:
URL: https://github.com/apache/spark/pull/28377#discussion_r416229855
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Column.scala
##########
@@ -1016,17 +1010,11 @@ class Column(val expr: Expression) extends Logging {
* df.select($"colA".name("colB"))
* }}}
*
- * If the current column has metadata associated with it, this metadata will be propagated
- * to the new column. If this not desired, use `as` with explicitly empty metadata.
Review comment:
@gatorsmile . From my understanding, it was because this is `inherited` from the child instead of `propagating`.
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620353423
**[Test build #121930 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121930/testReport)** for PR 28377 at commit [`bb09254`](https://github.com/apache/spark/commit/bb0925457c67294eeeffc63563435b48626d3f0d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] HeartSaVioR commented on a change in pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on a change in pull request #28377:
URL: https://github.com/apache/spark/pull/28377#discussion_r416230700
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Column.scala
##########
@@ -1016,17 +1010,11 @@ class Column(val expr: Expression) extends Logging {
* df.select($"colA".name("colB"))
* }}}
*
- * If the current column has metadata associated with it, this metadata will be propagated
- * to the new column. If this not desired, use `as` with explicitly empty metadata.
Review comment:
I'd say the comment is slightly invalid as of now, depending on how we interpret "propagation" here. If someone interprets propagation as a "copy" then that becomes wrong. It's more likely that we leave it on the behavior of Alias without touching.
And there're overloaded method which receives metadata, which may get rid of necessity of explaining.
But yes that doesn't mean we need to remove these comments - that's also an option we may reword a bit to try resolving ambiguity and keep leaving the part of comment instead.
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620406736
**[Test build #121936 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121936/testReport)** for PR 28377 at commit [`b6e410b`](https://github.com/apache/spark/commit/b6e410b68557b46324a3553c0777e9efe4694c98).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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] dongjoon-hyun commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620293161
Yes. Correct. In `name` function, the metadata loss occurs. Without the metadata, some structured query fails at Analysis Stage, and another structure queries becomes growing the internal group by state indefinitely.
----------------------------------------------------------------
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.
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] gatorsmile commented on a change in pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on a change in pull request #28377:
URL: https://github.com/apache/spark/pull/28377#discussion_r416226948
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Column.scala
##########
@@ -1016,17 +1010,11 @@ class Column(val expr: Expression) extends Logging {
* df.select($"colA".name("colB"))
* }}}
*
- * If the current column has metadata associated with it, this metadata will be propagated
- * to the new column. If this not desired, use `as` with explicitly empty metadata.
Review comment:
Anybody can tell me why we need to remove these comments?
----------------------------------------------------------------
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.
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 removed a comment on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620407764
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620283680
**[Test build #121930 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121930/testReport)** for PR 28377 at commit [`bb09254`](https://github.com/apache/spark/commit/bb0925457c67294eeeffc63563435b48626d3f0d).
----------------------------------------------------------------
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.
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 #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620407764
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620306258
**[Test build #121936 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121936/testReport)** for PR 28377 at commit [`b6e410b`](https://github.com/apache/spark/commit/b6e410b68557b46324a3553c0777e9efe4694c98).
----------------------------------------------------------------
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.
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] dongjoon-hyun commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620304478
Got it. 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.
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] HeartSaVioR commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620309415
> It sounds like the metadata is not available for TimeWindow expression when the function name(alias: String) copies the metadata to explicitMetadata. Thus, we do not set it when calling the APIs in Column.
That's right, but I don't see the reason of the fix only because we encountered an actual problem from there. This has been simply wrong because we set explicit metadata on Alias while it's not asked to overwrite. Javadoc of the Alias clearly describes the usage of explicitMetadata:
```
@param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
```
----------------------------------------------------------------
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.
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 #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620353939
----------------------------------------------------------------
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.
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 #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620284076
----------------------------------------------------------------
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.
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] dongjoon-hyun commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620282622
cc @cloud-fan , @zsxwing , @LiangchangZ , @xuanyuanking , @HeartSaVioR .
----------------------------------------------------------------
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.
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] dongjoon-hyun commented on a change in pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28377:
URL: https://github.com/apache/spark/pull/28377#discussion_r416240326
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Column.scala
##########
@@ -1016,17 +1010,11 @@ class Column(val expr: Expression) extends Logging {
* df.select($"colA".name("colB"))
* }}}
*
- * If the current column has metadata associated with it, this metadata will be propagated
- * to the new column. If this not desired, use `as` with explicitly empty metadata.
Review comment:
I'll recover them in 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.
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] dongjoon-hyun commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620300747
@gatorsmile . Are you assuming `NamedExpression` only?
----------------------------------------------------------------
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.
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] gatorsmile edited a comment on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
gatorsmile edited a comment on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620296526
@dongjoon-hyun @holdenk My understanding is different from what you said above.
It sounds like the metadata is not available for TimeWindow expression when the function `name(alias: String)` copies the metadata to `explicitMetadata`. Thus, we do not set it when calling the APIs in `Column`.
----------------------------------------------------------------
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.
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] dongjoon-hyun edited a comment on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun edited a comment on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620283666
cc @holdenk since she is the release manager of 2.4.6.
Also, cc @dbtsai
Without this, we cannot run long running Structure Streaming with State queries due to the lack of watermark.
----------------------------------------------------------------
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.
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 removed a comment on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620306513
----------------------------------------------------------------
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.
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] dongjoon-hyun commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-622020889
We have no viable solution for branch-2.4. You can proceed 2.4.6 release without 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.
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] gatorsmile commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620301600
Any other Expressions except `NamedExpression` has the field 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.
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] gatorsmile commented on a change in pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on a change in pull request #28377:
URL: https://github.com/apache/spark/pull/28377#discussion_r416232467
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Column.scala
##########
@@ -1016,17 +1010,11 @@ class Column(val expr: Expression) extends Logging {
* df.select($"colA".name("colB"))
* }}}
*
- * If the current column has metadata associated with it, this metadata will be propagated
- * to the new column. If this not desired, use `as` with explicitly empty metadata.
Review comment:
To me, it sounds like they are the same to our end users.
I do not have a strong opinion on this. Feel free to rewrite it. When writing the user-facing comments, please avoid explaining the implementation details. We just need to document the external impact and behaviors.
----------------------------------------------------------------
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.
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 removed a comment on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620284076
----------------------------------------------------------------
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.
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] SparkQA commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620283680
**[Test build #121930 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121930/testReport)** for PR 28377 at commit [`bb09254`](https://github.com/apache/spark/commit/bb0925457c67294eeeffc63563435b48626d3f0d).
----------------------------------------------------------------
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.
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] gatorsmile commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620296526
@dongjoon-hyun @holdenk My understanding is different from what you said above.
It sounds like the metadata is not available for TimeWindow expression when we copying the metadata to `explicitMetadata`. Thus, we do not set it when calling the APIs in `Column`.
----------------------------------------------------------------
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.
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] dongjoon-hyun commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620283666
cc @holdenk since she is the release manager of 2.4.6.
Without this, we cannot run long running Structure Streaming with State queries due to the lack of watermark.
----------------------------------------------------------------
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.
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] holdenk commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
holdenk commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-621957791
Are we still planning on backporting this or not? I see you deleted the branch so I'm assuming this particular attempt at backporting is abandoned but the root issue does seem important.
----------------------------------------------------------------
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.
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] dongjoon-hyun commented on a change in pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28377:
URL: https://github.com/apache/spark/pull/28377#discussion_r416229855
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Column.scala
##########
@@ -1016,17 +1010,11 @@ class Column(val expr: Expression) extends Logging {
* df.select($"colA".name("colB"))
* }}}
*
- * If the current column has metadata associated with it, this metadata will be propagated
- * to the new column. If this not desired, use `as` with explicitly empty metadata.
Review comment:
@gatorsmile . From my understanding, it was because this is `inherited` from the child instead of `propagating` with `explicitMetadata`.
----------------------------------------------------------------
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.
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 #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620306513
----------------------------------------------------------------
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.
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] gatorsmile commented on a change in pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
gatorsmile commented on a change in pull request #28377:
URL: https://github.com/apache/spark/pull/28377#discussion_r416230516
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/Column.scala
##########
@@ -1016,17 +1010,11 @@ class Column(val expr: Expression) extends Logging {
* df.select($"colA".name("colB"))
* }}}
*
- * If the current column has metadata associated with it, this metadata will be propagated
- * to the new column. If this not desired, use `as` with explicitly empty metadata.
Review comment:
I think we should keep them. The original comments are still valid. If users want to block metadata propagation, they should call the API `as(alias: String, metadata: Metadata):` with an empty 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.
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] HeartSaVioR commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
HeartSaVioR commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620314357
That said, I'd support porting back this to 2.4 as it fixes the wrong code. This change might impact broader audiences (so technically saying the PR/commit title doesn't represent actual change, sorry I should have found this earlier) hence the risk might be bigger than we may imagine, but I couldn't imagine the case which relies on the previous behavior (bug) to make it work. If we can imagine anything then it might be a signal to reconsider.
----------------------------------------------------------------
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.
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] SparkQA removed a comment on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620306258
**[Test build #121936 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121936/testReport)** for PR 28377 at commit [`b6e410b`](https://github.com/apache/spark/commit/b6e410b68557b46324a3553c0777e9efe4694c98).
----------------------------------------------------------------
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.
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] holdenk commented on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
holdenk commented on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620288941
From a backporting perspective it seems like this is maybe a bug given that `name` doesn't appear to be copying the metadata even without an explicit metadata set. Do I have the right context @dongjoon-hyun ?
----------------------------------------------------------------
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.
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 removed a comment on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620353939
----------------------------------------------------------------
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.
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] HeartSaVioR edited a comment on pull request #28377: [SPARK-27340][SS][2.4] Alias on TimeWindow expression may cause watermark metadata lost
Posted by GitBox <gi...@apache.org>.
HeartSaVioR edited a comment on pull request #28377:
URL: https://github.com/apache/spark/pull/28377#issuecomment-620309415
> It sounds like the metadata is not available for TimeWindow expression when the function name(alias: String) copies the metadata to explicitMetadata. Thus, we do not set it when calling the APIs in Column.
That's right, but I don't see the reason of the fix only because we encountered an actual problem from there. This has been simply wrong because we set explicit metadata on Alias while it's not asked to overwrite. Javadoc of the Alias clearly describes the usage of explicitMetadata:
```
@param explicitMetadata Explicit metadata associated with this alias that overwrites child's.
```
(that's why I couldn't agree with the initial proposal from #28326.)
----------------------------------------------------------------
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.
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