Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-32840][SQL][3.0] Invalid interval value can happen to be just adhesive with the unit #29716

Closed
wants to merge 3 commits into from

Conversation

yaooqinn
Copy link
Member

THIS PR backports #29708 to 3.0

What changes were proposed in this pull request?

In this PR, we add a checker for STRING form interval value ahead for parsing multiple units intervals and fail directly if the interval value contains alphabets to prevent correctness issues like interval '1 day 2' day=3 days.

Why are the changes needed?

fix correctness issue

Does this PR introduce any user-facing change?

yes, in spark 3.0.0 interval '1 day 2' day=3 days but now we fail with ParseException

How was this patch tested?

add a test.

@SparkQA
Copy link

SparkQA commented Sep 10, 2020

Test build #128523 has finished for PR 29716 at commit d3dc77c.

  • This patch fails Spark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 10, 2020

Test build #128524 has finished for PR 29716 at commit 3dd80b6.

  • This patch fails Spark unit tests.
  • This patch does not merge cleanly.
  • This patch adds no public classes.

@SparkQA
Copy link

SparkQA commented Sep 10, 2020

Test build #128526 has finished for PR 29716 at commit 6802215.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

maropu pushed a commit that referenced this pull request Sep 11, 2020
…adhesive with the unit

THIS PR backports #29708 to 3.0

### What changes were proposed in this pull request?
In this PR, we add a checker for STRING form interval value ahead for parsing multiple units intervals and fail directly if the interval value contains alphabets to prevent correctness issues like `interval '1 day 2' day`=`3 days`.

### Why are the changes needed?

fix correctness issue

### Does this PR introduce _any_ user-facing change?

yes, in spark 3.0.0 `interval '1 day 2' day`=`3 days` but now we fail with ParseException
### How was this patch tested?

add a test.

Closes #29716 from yaooqinn/SPARK-32840-30.

Authored-by: Kent Yao <yaooqinn@hotmail.com>
Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>
@maropu
Copy link
Member

maropu commented Sep 11, 2020

Thanks! Merged to branch-3.0.

@maropu maropu closed this Sep 11, 2020
@dongjoon-hyun
Copy link
Member

Thank you all. This happens in 3.0.0 too, right?

@cloud-fan
Copy link
Contributor

Yea, it's a bug starting from 3.0.0

holdenk pushed a commit to holdenk/spark that referenced this pull request Oct 27, 2020
…adhesive with the unit

THIS PR backports apache#29708 to 3.0

### What changes were proposed in this pull request?
In this PR, we add a checker for STRING form interval value ahead for parsing multiple units intervals and fail directly if the interval value contains alphabets to prevent correctness issues like `interval '1 day 2' day`=`3 days`.

### Why are the changes needed?

fix correctness issue

### Does this PR introduce _any_ user-facing change?

yes, in spark 3.0.0 `interval '1 day 2' day`=`3 days` but now we fail with ParseException
### How was this patch tested?

add a test.

Closes apache#29716 from yaooqinn/SPARK-32840-30.

Authored-by: Kent Yao <yaooqinn@hotmail.com>
Signed-off-by: Takeshi Yamamuro <yamamuro@apache.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants