Skip to content

Conversation

@MaxGekk
Copy link
Member

@MaxGekk MaxGekk commented Aug 10, 2022

What changes were proposed in this pull request?

In the PR, I propose to support casts of decimals to ANSI intervals, and follow the SQL standard:
173663908-71945980-5638-4b46-9020-4d2e4badef0c

Why are the changes needed?

To improve user experience with Spark SQL, and to conform to the SQL standard.

Does this PR introduce any user-facing change?

No, it just extends existing behavior of casts.

Before:

spark-sql> SELECT CAST(1.001002BD AS INTERVAL SECOND);
Error in query: cannot resolve 'CAST(1.001002BD AS INTERVAL SECOND)' due to data type mismatch: cannot cast decimal(7,6) to interval second; line 1 pos 7;

After:

spark-sql> SELECT CAST(1.001002BD AS INTERVAL SECOND);
0 00:00:01.001002000

How was this patch tested?

By running new tests:

$ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z cast.sql"
$ build/sbt "test:testOnly *CastWithAnsiOffSuite"
$ build/sbt "test:testOnly *CastWithAnsiOnSuite"

@github-actions github-actions bot added the SQL label Aug 10, 2022
@MaxGekk MaxGekk changed the title [WIP][SQL] Support cast of decimals to ANSI intervals [WIP][SPARK-40014][SQL] Support cast of decimals to ANSI intervals Aug 11, 2022
@MaxGekk MaxGekk marked this pull request as ready for review August 11, 2022 11:04
@MaxGekk MaxGekk changed the title [WIP][SPARK-40014][SQL] Support cast of decimals to ANSI intervals [SPARK-40014][SQL] Support cast of decimals to ANSI intervals Aug 11, 2022
@MaxGekk
Copy link
Member Author

MaxGekk commented Aug 12, 2022

@HyukjinKwon @cloud-fan @gengliangwang Could you review this PR, please.

d: Decimal, p: Int, s: Int, startField: Byte, endField: Byte): Int = {
try {
val months = if (endField == YEAR) d.toBigDecimal * MONTHS_PER_YEAR else d.toBigDecimal
months.setScale(0, BigDecimal.RoundingMode.HALF_UP).toIntExact
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have a sql reference doc for cast? we should mention the rounding behavior.

Copy link
Member Author

Choose a reason for hiding this comment

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

We have the doc for cast: https://spark.apache.org/docs/latest/sql-ref-ansi-compliance.html#cast but it says that Spark doesn't support cast of intervals to numerics. Let me open a PR and document recent changes in casting of ANSI intervals.

Copy link
Member Author

Choose a reason for hiding this comment

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

@cloud-fan I opened the PR #37495 which updates the doc.

@MaxGekk MaxGekk closed this in 3f84c7d Aug 12, 2022
@MaxGekk
Copy link
Member Author

MaxGekk commented Aug 12, 2022

Merging to master. Thank you, @cloud-fan for review.

MaxGekk added a commit that referenced this pull request Aug 13, 2022
### What changes were proposed in this pull request?
In the PR, I propose to update the doc page https://spark.apache.org/docs/latest/sql-ref-ansi-compliance.html#cast about new behaviour of CAST introduced by the following PRs:
1. [[SPARK-40008][SQL] Support casting of integrals to ANSI intervals](#37442)
2. [[SPARK-39451][SQL] Support casting intervals to integrals in ANSI mode](#36811)
3. [[SPARK-39470][SQL] Support cast of ANSI intervals to decimals](#36857)
4. [[SPARK-40014][SQL] Support cast of decimals to ANSI intervals](#37466)
<img width="1004" alt="Screenshot 2022-08-12 at 15 00 10" src="https://user-images.githubusercontent.com/1580697/184349887-700c64f6-e9c8-40c4-85cc-a77072506cf5.png">

### Why are the changes needed?
To inform Spark SQL about new behaviour of `CAST`.

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

### How was this patch tested?
By building Spark SQL docs and check them manually:
```
$ SKIP_SCALADOC=1 SKIP_PYTHONDOC=1 SKIP_RDOC=1 bundle exec jekyll build
```

Closes #37495 from MaxGekk/doc-interval-cast.

Authored-by: Max Gekk <[email protected]>
Signed-off-by: Max Gekk <[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.

2 participants