[SPARK-39567][SQL] Support ANSI intervals in the percentile functions #36965
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@cloud-fan Are you ok with this approach? |
cloud-fan
reviewed
Jun 24, 2022
...atalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/percentiles.scala
Outdated
Show resolved
Hide resolved
|
this approach LGTM |
MaxGekk
changed the title
|
I guess this is not related to changes: |
cloud-fan
approved these changes
Jun 27, 2022
|
Merging to master. Thank you, @cloud-fan for review. |
MaxGekk
added a commit
that referenced
this pull request
Aug 22, 2022
…le functions ### What changes were proposed in this pull request? In the PR, I propose to change the result type of the `PercentileBase` (the `Percentile`, `PercentileDisc`, `Median`, `PercentileCont` expressions): - For any year-month interval types return `YEAR TO MONTH INTERVAL` - For any day-time interval types return `DAY TO SECOND INTERVAL` The PR changes behavior of the following functions: `median()`, `percentile()`, `percentile_cont()` and `percentile_disc()`. This PR is a follow up of #36965. ### Why are the changes needed? 1. To not loose the fraction part of the result when it is possible. To behave in the same way as for numeric types when the functions return double. 2. To be consistent with `MEAN` which returns wide ANSI types, for instance: ```sql spark-sql> SELECT typeof(mean(distinct cast(col as interval hour))) FROM VALUES (1), (2), (2), (3), (4), (NULL) AS tab(col); interval day to second ``` ### Does this PR introduce _any_ user-facing change? Yes. Before: ```sql spark-sql> SELECT typeof(median(distinct cast(col as interval hour))) FROM VALUES (1), (2), (2), (3), (4), (NULL) AS tab(col); interval hour ``` After: ```sql spark-sql> SELECT typeof(median(distinct cast(col as interval hour))) FROM VALUES (1), (2), (2), (3), (4), (NULL) AS tab(col); interval day to second ``` ### How was this patch tested? By running new tests: ``` $ build/sbt "sql/testOnly org.apache.spark.sql.SQLQueryTestSuite -- -z percentiles.sql" ``` Closes #37595 from MaxGekk/median-result-ansi-intervals. Authored-by: Max Gekk <max.gekk@gmail.com> Signed-off-by: Max Gekk <max.gekk@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
In the PR, I propose to extend the
PercentileBaseto support ANSI interval types (year-month and day-time interval types) by thePercentile,PercentileDisc,Median,PercentileContexpressions. Intermediately, input intervals are casted to double values, and eventually the result of the calculations are casted back to intervals.Why are the changes needed?
To improve user experience with Spark SQL.
Does this PR introduce any user-facing change?
No. Before the changes, the
percentilefunctions fail with error:After:
How was this patch tested?
By running the modified tests: