[SPARK-28389][SQL] Use Java 8 API in add_months #25153

MaxGekk · 2019-07-14T18:30:41Z

What changes were proposed in this pull request?

In the PR, I propose to use the plusMonths() method of LocalDate to add months to a date. This method adds the specified amount to the months field of LocalDate in three steps:

Add the input months to the month-of-year field
Check if the resulting date would be invalid
Adjust the day-of-month to the last valid day if necessary

The difference between current behavior and propose one is in handling the last day of month in the original date. For example, adding 1 month to 2019-02-28 will produce 2019-03-28 comparing to the current implementation where the result is 2019-03-31.

The proposed behavior is implemented in MySQL and PostgreSQL.

How was this patch tested?

By existing test suites DateExpressionsSuite, DateFunctionsSuite and DateTimeUtilsSuite.

MaxGekk · 2019-07-14T18:31:23Z

@cloud-fan Please, take a look at the PR.

srowen

I favor this change as less surprising and more consistent with Java APIs too.

srowen · 2019-07-14T19:43:36Z

docs/sql-migration-guide-upgrade.md


  - Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.

+  - Since Spark 3.0, the `add_months` function does not adjust the resulting date to a last day of month if the original date is a last day of month. The resulting date is adjust to a last day of month only if it is invalid. For example, `select add_months(DATE'2019-02-28', 1)` produces `2019-03-28` but `select add_months(DATE'2019-01-31', 1)` produces `2019-02-28`.


is adjust -> is adjusted.
So previously, adding a month to 2019-02-28 resulted in 2019-03-31? It might be worth clarifying that too in your example.

So previously, adding a month to 2019-02-28 resulted in 2019-03-31?

Yes, it does:

scala> spark.sql("select add_months(DATE'2019-02-28', 1)").show +--------------------------------+ |add_months(DATE '2019-02-28', 1)| +--------------------------------+ | 2019-03-31| +--------------------------------+

I updated the SQL migration guide, and add the example.

SparkQA · 2019-07-14T19:57:39Z

Test build #107649 has finished for PR 25153 at commit 668608c.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
final class AddColumn implements ColumnChange
final class RenameColumn implements ColumnChange
final class UpdateColumnType implements ColumnChange
final class UpdateColumnComment implements ColumnChange
final class DeleteColumn implements ColumnChange
case class UnresolvedRelation(
case class AlterTable(
case class AlterTableExec(

MaxGekk · 2019-07-14T20:35:41Z

The latest build failed on CollectionExpressionsSuite, in particular on:

      checkEvaluation(new Sequence(
        Literal(Date.valueOf("2018-01-31")),
        Literal(Date.valueOf("2018-04-30")),
        Literal(CalendarInterval.fromString("interval 1 month"))),
        Seq(
          Date.valueOf("2018-01-31"),
          Date.valueOf("2018-02-28"),
          Date.valueOf("2018-03-31"),
          Date.valueOf("2018-04-30")))

because current implementation incrementally adds steps of month and micros to previous value. So, such implementation produces:

Seq(
          Date.valueOf("2018-01-31"),
          Date.valueOf("2018-02-28"),
          Date.valueOf("2018-03-28"),
          Date.valueOf("2018-04-28"))

I am going to change this in TemporalSequenceImpl to arr[i+1] = add_monts(start, i * stepMonth, i * stepMicros, timeZone)

SparkQA · 2019-07-14T23:57:43Z

Test build #107652 has finished for PR 25153 at commit f1a4241.

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

cloud-fan · 2019-07-15T02:26:42Z

      checkEvaluation(new Sequence(
        Literal(Date.valueOf("2018-01-31")),
        Literal(Date.valueOf("2018-04-30")),
        Literal(CalendarInterval.fromString("interval 1 month"))),
        Seq(
          Date.valueOf("2018-01-31"),
          Date.valueOf("2018-02-28"),
          Date.valueOf("2018-03-31"),
          Date.valueOf("2018-04-30")))

Is this a valid test? Do other databases have the same behavior?

MaxGekk · 2019-07-15T08:01:21Z

@cloud-fan This feature was borrowed from prestodb #21155 . Looking at the issue prestodb/presto#10765 , the expected values in the test are valid, it seems.

wangyum · 2019-07-15T08:20:11Z

Teradata also returns 2019-03-28 if add a month to 2019-02-28:

cloud-fan · 2019-07-15T12:50:06Z

thanks, merging to master!

## What changes were proposed in this pull request? In the PR, I propose to use the `plusMonths()` method of `LocalDate` to add months to a date. This method adds the specified amount to the months field of `LocalDate` in three steps: 1. Add the input months to the month-of-year field 2. Check if the resulting date would be invalid 3. Adjust the day-of-month to the last valid day if necessary The difference between current behavior and propose one is in handling the last day of month in the original date. For example, adding 1 month to `2019-02-28` will produce `2019-03-28` comparing to the current implementation where the result is `2019-03-31`. The proposed behavior is implemented in MySQL and PostgreSQL. ## How was this patch tested? By existing test suites `DateExpressionsSuite`, `DateFunctionsSuite` and `DateTimeUtilsSuite`. Closes apache#25153 from MaxGekk/add-months. Authored-by: Maxim Gekk <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

HyukjinKwon · 2019-07-19T03:45:20Z

docs/sql-migration-guide-upgrade.md


  - Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.

+  - Since Spark 3.0, the `add_months` function adjusts the resulting date to a last day of month only if it is invalid. For example, `select add_months(DATE'2019-01-31', 1)` results `2019-02-28`. In Spark version 2.4 and earlier, the resulting date is adjusted when it is invalid, or the original date is a last day of months. For example, adding a month to `2019-02-28` resultes in `2019-03-31`.


Hm .. shall we update this migration guide? Actually select add_months(DATE'2019-01-31', 1) returns 2019-02-28 in both the current master and the versions before Spark 2.4.x.

I think we should explicitly mention select add_months(DATE'2019-02-28', 1) case only:

scala> sql("select add_months(DATE'2019-02-28', 1)").show() +--------------------------------+ |add_months(DATE '2019-02-28', 1)| +--------------------------------+ | 2019-03-31| +--------------------------------+

scala> sql("select add_months(DATE'2019-02-28', 1)").show() +--------------------------------+ |add_months(DATE '2019-02-28', 1)| +--------------------------------+ | 2019-03-28| +--------------------------------+

Let me update it soon.

MaxGekk added 5 commits July 11, 2019 22:50

Use Java 8 API to add months

49061b9

Adjust tests

93a16ae

Remove unused methods

746de08

Update the SQL migration guide

42d8e2d

Merge remote-tracking branch 'origin/master' into add-months

668608c

srowen reviewed Jul 14, 2019

View reviewed changes

MaxGekk added 2 commits July 14, 2019 23:37

Calc next elem as add_months(start, i * stepMonth, i * stepMicro)

5294204

Improve the SQL migration guide.

f1a4241

cloud-fan closed this in f241fc7 Jul 15, 2019

HyukjinKwon reviewed Jul 19, 2019

View reviewed changes

HyukjinKwon mentioned this pull request Jul 19, 2019

[SPARK-28389][SQL][FOLLOW-UP] Use one example in 'add_months' behavior change at migration guide #25199

Closed

MaxGekk deleted the add-months branch September 18, 2019 15:58

dongjoon-hyun added the SQL label Feb 5, 2020

Amalicia mentioned this pull request Jul 8, 2022

Added Sorting Functions hablapps/doric#233

Merged

1 task


		- Since Spark 3.0, substitution order of nested WITH clauses is changed and an inner CTE definition takes precedence over an outer. In version 2.4 and earlier, `WITH t AS (SELECT 1), t2 AS (WITH t AS (SELECT 2) SELECT * FROM t) SELECT * FROM t2` returns `1` while in version 3.0 it returns `2`. The previous behaviour can be restored by setting `spark.sql.legacy.ctePrecedence.enabled` to `true`.

		- Since Spark 3.0, the `add_months` function does not adjust the resulting date to a last day of month if the original date is a last day of month. The resulting date is adjust to a last day of month only if it is invalid. For example, `select add_months(DATE'2019-02-28', 1)` produces `2019-03-28` but `select add_months(DATE'2019-01-31', 1)` produces `2019-02-28`.

[SPARK-28389][SQL] Use Java 8 API in add_months #25153

[SPARK-28389][SQL] Use Java 8 API in add_months #25153

Uh oh!

Conversation

MaxGekk commented Jul 14, 2019

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

MaxGekk commented Jul 14, 2019

Uh oh!

srowen left a comment

Choose a reason for hiding this comment

Uh oh!

srowen Jul 14, 2019

Choose a reason for hiding this comment

Uh oh!

MaxGekk Jul 14, 2019

Choose a reason for hiding this comment

Uh oh!

MaxGekk Jul 14, 2019

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jul 14, 2019

Uh oh!

MaxGekk commented Jul 14, 2019

Uh oh!

SparkQA commented Jul 14, 2019

Uh oh!

cloud-fan commented Jul 15, 2019

Uh oh!

MaxGekk commented Jul 15, 2019

Uh oh!

wangyum commented Jul 15, 2019

Uh oh!

cloud-fan commented Jul 15, 2019

Uh oh!

HyukjinKwon Jul 19, 2019

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Jul 19, 2019

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants