Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Jul 15, 2019

What changes were proposed in this pull request?

Existing random generators in tests produce wide ranges of values that can be out of supported ranges for:

  • DateType, the valid range is [0001-01-01, 9999-12-31]
  • TimestampType supports values in [0001-01-01T00:00:00.000000Z, 9999-12-31T23:59:59.999999Z]
  • CalendarIntervalType should define intervals for the ranges above.

Dates and timestamps produced by random literal generators are usually out of valid ranges for those types. And tests just check invalid values or values caused by arithmetic overflow.

In the PR, I propose to restrict tested pseudo-random values by valid ranges of DateType, TimestampType and CalendarIntervalType. This should allow to check valid values in test, and avoid wasting time on a priori invalid inputs.

How was this patch tested?

The changes were checked by DateExpressionsSuite and modified DateTimeUtils.dateAddMonths:

  def dateAddMonths(days: SQLDate, months: Int): SQLDate = {
    localDateToDays(LocalDate.ofEpochDay(days).plusMonths(months))
  }

@SparkQA
Copy link

SparkQA commented Jul 16, 2019

Test build #107704 has finished for PR 25166 at commit c4cea83.

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

@dongjoon-hyun
Copy link
Member

Merged to master. Thank you, @MaxGekk and @HyukjinKwon .

vinodkc pushed a commit to vinodkc/spark that referenced this pull request Jul 18, 2019
…Type and CalendarIntervalType

## What changes were proposed in this pull request?

Existing random generators in tests produce wide ranges of values that can be out of supported ranges for:
- `DateType`, the valid range is `[0001-01-01, 9999-12-31]`
- `TimestampType` supports values in `[0001-01-01T00:00:00.000000Z, 9999-12-31T23:59:59.999999Z]`
- `CalendarIntervalType` should define intervals for the ranges above.

Dates and timestamps produced by random literal generators are usually out of valid ranges for those types. And tests just check invalid values or values caused by arithmetic overflow.

In the PR, I propose to restrict tested pseudo-random values by valid ranges of `DateType`, `TimestampType` and `CalendarIntervalType`. This should allow to check valid values in test, and avoid wasting time on a priori invalid inputs.

## How was this patch tested?

The changes were checked by `DateExpressionsSuite` and modified `DateTimeUtils.dateAddMonths`:
```Scala
  def dateAddMonths(days: SQLDate, months: Int): SQLDate = {
    localDateToDays(LocalDate.ofEpochDay(days).plusMonths(months))
  }
```

Closes apache#25166 from MaxGekk/datetime-lit-random-gen.

Authored-by: Maxim Gekk <[email protected]>
Signed-off-by: Dongjoon Hyun <[email protected]>
@MaxGekk MaxGekk deleted the datetime-lit-random-gen branch September 18, 2019 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants