Uh oh!
There was an error while loading. Please reload this page.
[SPARK-26379][SS][BRANCH-2.3] Use dummy TimeZoneId to avoid UnresolvedException in CurrentBatchTimestamp - #23656
Conversation
…to streaming query ## What changes were proposed in this pull request? This patch proposes to fix issue on adding `current_timestamp` / `current_date` with streaming query. The root reason is that Spark transforms `CurrentTimestamp`/`CurrentDate` to `CurrentBatchTimestamp` in MicroBatchExecution which makes transformed attributes not-yet-resolved. They will be resolved by IncrementalExecution. (In ContinuousExecution, Spark doesn't allow using `current_timestamp` and `current_date` so it has been OK.) It's OK for DataSource V1 sink because it simply leverages transformed logical plan and don't evaluate until they're resolved, but for DataSource V2 sink, Spark tries to extract the schema of transformed logical plan in prior to IncrementalExecution, and unresolved attributes will raise errors. This patch fixes the issue via having separate pre-resolved logical plan to pass the schema to StreamingWriteSupport safely. ## How was this patch tested? Added UT. Closesapache#23609 from HeartSaVioR/SPARK-26379. Authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan@gmail.com> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
dongjoon-hyun
commented
Jan 25, 2019
Thank you, @HeartSaVioR . :) |
SparkQA
commented
Jan 26, 2019
Test build #101696 has finished for PR 23656 at commit
|
dongjoon-hyun
commented
Jan 26, 2019
We will revisit this PR because the original PR needs a more verification and a followup PR is expected. |
HeartSaVioR
commented
Jan 26, 2019
FYI: follow-up PR is available in #23660 submitted by Dongjoon. |
dongjoon-hyun
commented
Jan 27, 2019
Hi, @HeartSaVioR . Could you update this PR with the final content? |
…xception in CurrentBatchTimestamp ## What changes were proposed in this pull request? Spark replaces `CurrentTimestamp` with `CurrentBatchTimestamp`. However, `CurrentBatchTimestamp` is `TimeZoneAwareExpression` while `CurrentTimestamp` isn't. Without TimeZoneId, `CurrentBatchTimestamp` becomes unresolved and raises `UnresolvedException`. Since `CurrentDate` is `TimeZoneAwareExpression`, there is no problem with `CurrentDate`. This PR reverts the [previous patch](apache#23609) on `MicroBatchExecution` and fixes the root cause. ## How was this patch tested? Pass the Jenkins with the updated test cases. Closesapache#23660 from dongjoon-hyun/SPARK-26379. Authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
HeartSaVioR
commented
Jan 27, 2019
@dongjoon-hyun Yes, I've just cherry-picked the FOLLOW-UP commit into PR branch. |
dongjoon-hyun
commented
Jan 27, 2019
Thank you, @HeartSaVioR . It's enough since this will be merged as a single commit. |
dongjoon-hyun
commented
Jan 27, 2019
@HeartSaVioR . Could you update the PR title and description, too? |
SparkQA
commented
Jan 28, 2019
Test build #101734 has finished for PR 23656 at commit
|
HeartSaVioR
commented
Jan 28, 2019
@dongjoon-hyun Sorry for the late. Just updated the title and content of PR. |
HeartSaVioR
commented
Jan 28, 2019
Test failure looks not relevant. Retest this, please. |
maropu
commented
Jan 28, 2019
yea, that's known flaky tests in branch-2.3.. |
SparkQA
commented
Jan 28, 2019
Test build #101740 has finished for PR 23656 at commit
|
maropu
commented
Jan 28, 2019
retest this please |
dongjoon-hyun
commented
Jan 28, 2019
Thank you, @HeartSaVioR and @maropu . |
dongjoon-hyun
left a comment
There was a problem hiding this comment.
+1, LGTM (pending Jenkins).
SparkQA
commented
Jan 28, 2019
Test build #101743 has finished for PR 23656 at commit
|
dongjoon-hyun
commented
Jan 28, 2019
Retest this please. |
SparkQA
commented
Jan 28, 2019
Test build #101747 has finished for PR 23656 at commit
|
HeartSaVioR
commented
Jan 28, 2019
retest this, please |
SparkQA
commented
Jan 28, 2019
Test build #101749 has finished for PR 23656 at commit
|
HeartSaVioR
commented
Jan 28, 2019
Retest this please |
maropu
commented
Jan 28, 2019
super flaky.... |
HeartSaVioR
commented
Jan 28, 2019
Yeah... If the branch should be maintained further, it may need to fix the flaky tests... As it seems to be known flakiness, are we aware about the fix in upper version lines? |
SparkQA
commented
Jan 28, 2019
Test build #101758 has finished for PR 23656 at commit
|
dongjoon-hyun
commented
Jan 28, 2019
Retest this please. |
2 similar comments
dongjoon-hyun
commented
Jan 28, 2019
Retest this please. |
dongjoon-hyun
commented
Jan 28, 2019
Retest this please. |
dongjoon-hyun
commented
Jan 28, 2019
@HeartSaVioR and @maropu . I triggered Jenkins three times intentionally to pass the Jenkins. |
SparkQA
commented
Jan 28, 2019
Test build #101765 has finished for PR 23656 at commit
|
SparkQA
commented
Jan 28, 2019
Test build #101764 has finished for PR 23656 at commit
|
SparkQA
commented
Jan 28, 2019
Test build #101763 has finished for PR 23656 at commit
|
2 failures, 1 passes. It looks like kind of serious one if Spark community will maintain branch-2.3 forward. |
dongjoon-hyun
commented
Jan 28, 2019
@HeartSaVioR . Yes. It's true and |
dongjoon-hyun
commented
Jan 28, 2019
Merged to branch-2.3. |
…dException in CurrentBatchTimestamp ## What changes were proposed in this pull request? Spark replaces `CurrentTimestamp` with `CurrentBatchTimestamp`. However, `CurrentBatchTimestamp` is `TimeZoneAwareExpression` while `CurrentTimestamp` isn't. Without TimeZoneId, `CurrentBatchTimestamp` becomes unresolved and raises `UnresolvedException`. Since `CurrentDate` is `TimeZoneAwareExpression`, there is no problem with `CurrentDate`. ## How was this patch tested? Pass the Jenkins with the updated test cases. Closes#23656 from HeartSaVioR/SPARK-26379-branch-2.3. Lead-authored-by: Jungtaek Lim (HeartSaVioR) <kabhwan@gmail.com> Co-authored-by: Dongjoon Hyun <dongjoon@apache.org> Signed-off-by: Dongjoon Hyun <dongjoon@apache.org>
maropu
commented
Jan 29, 2019
@HeartSaVioR I'm not familiar with the SS part though, your other pr (#23634) is not related to the failures? It seems the failures are of the outer joins tests... |
HeartSaVioR
commented
Jan 29, 2019
If #23634 was the fix for tests they would be flaky for branch-2.4 / master as well. They don't seem to be flaky in master branch. |
maropu
commented
Jan 29, 2019
Aha, I see. |
What changes were proposed in this pull request?
Spark replaces
CurrentTimestampwithCurrentBatchTimestamp.However,
CurrentBatchTimestampisTimeZoneAwareExpressionwhileCurrentTimestampisn't.Without TimeZoneId,
CurrentBatchTimestampbecomes unresolved and raisesUnresolvedException.Since
CurrentDateisTimeZoneAwareExpression, there is no problem withCurrentDate.How was this patch tested?
Pass the Jenkins with the updated test cases.