Skip to content

[SPARK-29190][SQL] Optimize extract/date_part for the milliseconds field #25871

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
MaxGekk wants to merge 5 commits into from
Closed

[SPARK-29190][SQL] Optimize extract/date_part for the milliseconds field #25871

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7cf1082
Avoid decimal div in getting milliseconds
MaxGekk Sep 20, 2019
53e57ef
Regen results of ExtractBenchmark
MaxGekk Sep 20, 2019
99673d9
Add jdk8 result
dongjoon-hyun Sep 21, 2019
1e125cf
Add jdk11 result
dongjoon-hyun Sep 21, 2019
fb64679
Merge pull request #23 from dongjoon-hyun/PR-25871
MaxGekk Sep 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/util/DateTimeUtils.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -465,8 +465,7 @@ object DateTimeUtils {
* is expressed in microseconds since the epoch.
*/
def getMilliseconds(timestamp: SQLTimestamp, timeZone: TimeZone): Decimal = {
val micros = Decimal(getMicroseconds(timestamp, timeZone))
(micros / Decimal(MICROS_PER_MILLIS)).toPrecision(8, 3)
Decimal(getMicroseconds(timestamp, timeZone), 8, 3)

@HyukjinKwon HyukjinKwon Sep 21, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@MaxGekk, is it safe? Seems previously it could return null but now we cannot return null in some conditions. For instance:

scala> Decimal(9223372036854775L)
res27: org.apache.spark.sql.types.Decimal = 9223372036854775

scala> Decimal(9223372036854775L, 8, 3)
java.lang.ArithmeticException: Unscaled value too large for precision
  at org.apache.spark.sql.types.Decimal.set(Decimal.scala:79)
  at org.apache.spark.sql.types.Decimal$.apply(Decimal.scala:564)
  ... 49 elided

whereas toPrecision seems able to return null for overflow.

@MaxGekk MaxGekk Sep 21, 2019

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I think so, getMicroseconds returns an int in the range [0, 60000000) for which Decimal(..., 8, 3) is always valid, for example:

scala> Decimal(60000000, 8, 3)
res1: org.apache.spark.sql.types.Decimal = 60000.000

@HyukjinKwon HyukjinKwon Sep 21, 2019

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Oh, yes. I checked other cases too and seems fine.

}

/**
Expand Down
Loading