Skip to content

Conversation

@TJX2014
Copy link
Contributor

@TJX2014 TJX2014 commented Jun 20, 2020

What changes were proposed in this pull request?

  1. Add judge equal as bigger condition in org.apache.spark.sql.catalyst.expressions.Sequence.TemporalSequenceImpl#eval
  2. Unit test for interval day, month, year
    Former PR to master is https://github.com/apache/spark/pull/28819
    Current PR change stringToInterval => CalendarInterval.fromString in order to compatible with branch-2.4

Why are the changes needed?

Bug exists when sequence input get same equal start and end dates, which will occur while loop forever

Does this PR introduce any user-facing change?

Yes,
Before this PR, people will get a java.lang.ArrayIndexOutOfBoundsException, when eval as below:
sql("select sequence(cast('2011-03-01' as date), cast('2011-03-01' as date), interval 1 year)").show(false)

How was this patch tested?

Unit test.

@TJX2014
Copy link
Contributor Author

TJX2014 commented Jun 20, 2020

Hi @dongjoon-hyun
Could you please check this PR for branch-2.4 which paired with https://github.com/apache/spark/pull/28819

@maropu
Copy link
Member

maropu commented Jun 20, 2020

Is this not follow-up but backport, right?

@maropu
Copy link
Member

maropu commented Jun 20, 2020

ok to test

@maropu maropu changed the title [SPARK-31980][SQL][FOLLOW-UP]Function sequence() fails if start and end of range are equal dates [SPARK-31980][SQL][2.4]Function sequence() fails if start and end of range are equal dates Jun 20, 2020
@maropu maropu changed the title [SPARK-31980][SQL][2.4]Function sequence() fails if start and end of range are equal dates [SPARK-31980][SQL][2.4] Function sequence() fails if start and end of range are equal dates Jun 20, 2020
@TJX2014
Copy link
Contributor Author

TJX2014 commented Jun 20, 2020

Is this not follow-up but backport, right?

Yes, it is a backport.

@SparkQA
Copy link

SparkQA commented Jun 20, 2020

Test build #124318 has finished for PR 28877 at commit 601e93b.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Thanks, @TJX2014 and @maropu .
Merged to branch-2.4.

dongjoon-hyun pushed a commit that referenced this pull request Jun 20, 2020
… range are equal dates

### What changes were proposed in this pull request?
1. Add judge equal as bigger condition in `org.apache.spark.sql.catalyst.expressions.Sequence.TemporalSequenceImpl#eval`
2. Unit test for interval `day`, `month`, `year`
Former PR to master is [https://github.com/apache/spark/pull/28819](url)
Current PR change `stringToInterval => CalendarInterval.fromString` in order to compatible with branch-2.4

### Why are the changes needed?
Bug exists when sequence input get same equal start and end dates, which will occur `while loop` forever

### Does this PR introduce _any_ user-facing change?
Yes,
Before this PR, people will get a `java.lang.ArrayIndexOutOfBoundsException`, when eval as below:
`sql("select sequence(cast('2011-03-01' as date), cast('2011-03-01' as date), interval 1 year)").show(false)
`

### How was this patch tested?
Unit test.

Closes #28877 from TJX2014/branch-2.4-SPARK-31980-compatible.

Authored-by: TJX2014 <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants