[SPARK-17174][SQL] Add the support for TimestampType for some functions as output type #14788
There are no files selected for viewing
This file contains hidden or 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
This file contains hidden or 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
This file contains hidden or 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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,58 +72,64 @@ case class CurrentTimestamp() extends LeafExpression with CodegenFallback { | |
| } | ||
|
|
||
| /** | ||
| * The base for addition/subtraction for days. | ||
|
Member
Author
There was a problem hiding this comment. it seems it shows the diff a bit messy here. I made a common parent for |
||
| */ | ||
| abstract class AddDaysBase(instant: Expression, days: Expression) | ||
| extends BinaryExpression with ImplicitCastInputTypes { | ||
|
|
||
| override def left: Expression = instant | ||
| override def right: Expression = days | ||
|
|
||
| override def inputTypes: Seq[AbstractDataType] = | ||
| Seq(TypeCollection(DateType, TimestampType), IntegerType) | ||
|
|
||
| override def dataType: DataType = instant.dataType | ||
|
|
||
| // 1 for addition, -1 for subtraction | ||
| def signModifier: Int | ||
|
|
||
| override def nullSafeEval(start: Any, days: Any): Any = { | ||
| (instant.dataType, start, days) match { | ||
| case (_: DateType, startDate: Int, days: Int) => | ||
| startDate + (signModifier * days) | ||
| case (_: TimestampType, startTimestamp: Long, days: Int) => | ||
| DateTimeUtils.timestampAddDays(startTimestamp, signModifier * days) | ||
| } | ||
| } | ||
|
|
||
| override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { | ||
| instant.dataType match { | ||
| case DateType => | ||
| nullSafeCodeGen(ctx, ev, (sd, d) => s"""${ev.value} = $sd + ($signModifier * $d);""") | ||
| case TimestampType => | ||
| val dtu = DateTimeUtils.getClass.getName.stripSuffix("$") | ||
| defineCodeGen(ctx, ev, (sd, d) => s"""$dtu.timestampAddDays($sd, ($signModifier * $d))""") | ||
| } | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Adds a number of days to date/timestamp. | ||
| */ | ||
| @ExpressionDescription( | ||
| usage = "_FUNC_(instant, num_days) - Returns the date/timestamp that is num_days after instant.", | ||
| extended = "> SELECT _FUNC_('2016-07-30', 1);\n '2016-07-31'") | ||
| case class AddDays(instant: Expression, days: Expression) extends AddDaysBase(instant, days) { | ||
|
|
||
| override def signModifier: Int = 1 | ||
|
|
||
| override def prettyName: String = "date_add" | ||
| } | ||
|
|
||
| /** | ||
| * Subtracts a number of days to date/timestamp. | ||
| */ | ||
| @ExpressionDescription( | ||
| usage = "_FUNC_(instant, num_days) - Returns the date/timestamp that is num_days before instant.", | ||
| extended = "> SELECT _FUNC_('2016-07-30', 1);\n '2016-07-29'") | ||
| case class DateSub(instant: Expression, days: Expression) extends AddDaysBase(instant, days) { | ||
|
|
||
| override def signModifier: Int = -1 | ||
|
|
||
| override def prettyName: String = "date_sub" | ||
| } | ||
|
|
@@ -781,29 +787,38 @@ case class TimeSub(start: Expression, interval: Expression) | |
| } | ||
|
|
||
| /** | ||
| * Returns the date/timestamp that is num_months after instant. | ||
| */ | ||
| // scalastyle:off line.size.limit | ||
| @ExpressionDescription( | ||
| usage = "_FUNC_(instant, num_months) - Returns the date/timestamp that is num_months after instant.", | ||
| extended = "> SELECT _FUNC_('2016-08-31', 1);\n '2016-09-30'") | ||
| // scalastyle:on line.size.limit | ||
| case class AddMonths(instant: Expression, numMonths: Expression) | ||
| extends BinaryExpression with ImplicitCastInputTypes { | ||
|
|
||
| override def left: Expression = instant | ||
| override def right: Expression = numMonths | ||
|
|
||
| override def inputTypes: Seq[AbstractDataType] = | ||
| Seq(TypeCollection(DateType, TimestampType), IntegerType) | ||
|
|
||
| override def dataType: DataType = instant.dataType | ||
|
|
||
| override def nullSafeEval(start: Any, months: Any): Any = { | ||
| (instant.dataType, start, months) match { | ||
| case (_: DateType, startDate: Int, months: Int) => | ||
| DateTimeUtils.dateAddMonths(startDate, months) | ||
| case (_: TimestampType, startTimestamp: Long, months: Int) => | ||
| DateTimeUtils.timestampAddInterval(startTimestamp, months, 0) | ||
| } | ||
| } | ||
|
|
||
| override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { | ||
| val dtu = DateTimeUtils.getClass.getName.stripSuffix("$") | ||
| defineCodeGen(ctx, ev, (sd, m) => instant.dataType match { | ||
| case DateType => s"""$dtu.dateAddMonths($sd, $m)""" | ||
| case TimestampType => s"""$dtu.timestampAddInterval($sd, $m, 0)""" | ||
| }) | ||
| } | ||
|
|
||
|
|
@@ -916,21 +931,26 @@ case class ToDate(child: Expression) extends UnaryExpression with ImplicitCastIn | |
| } | ||
|
|
||
| /** | ||
| * Returns timestamp truncated to the unit specified by the format. | ||
| */ | ||
| // scalastyle:off line.size.limit | ||
| @ExpressionDescription( | ||
| usage = "_FUNC_(instant, fmt) - Returns returns date/timestamp with the time portion truncated to the unit specified by the format model fmt.", | ||
| extended = "> SELECT _FUNC_('2009-02-12', 'MM')\n '2009-02-01 00:00:00'\n> SELECT _FUNC_('2015-10-27', 'YEAR');\n '2015-01-01 00:00:00'") | ||
| // scalastyle:on line.size.limit | ||
| case class TruncInstant(instant: Expression, format: Expression) | ||
|
Contributor
There was a problem hiding this comment. This auto casts. I think we are still breaking API here if a user passes a Timestamp. In the old situation the user would always get a Date, and now he gets a Date or Timestamp based on the input type. So I think we need to split this into two expressions.
Member
Author
There was a problem hiding this comment. I misunderstood the first comment.. Will make two expressions. Thanks!
Member
Author
There was a problem hiding this comment. @hvanhovell ur.. actually, should I split other functions I corrected here as well here? |
||
| extends BinaryExpression with ImplicitCastInputTypes { | ||
|
|
||
| override def left: Expression = instant | ||
| override def right: Expression = format | ||
|
|
||
| override def inputTypes: Seq[AbstractDataType] = | ||
| Seq(TypeCollection(DateType, TimestampType), StringType) | ||
|
|
||
| override def dataType: DataType = instant.dataType | ||
|
|
||
| override def nullable: Boolean = true | ||
|
|
||
| override def prettyName: String = "trunc" | ||
|
|
||
| private lazy val truncLevel: Int = | ||
|
|
@@ -946,11 +966,12 @@ case class TruncDate(date: Expression, format: Expression) | |
| // unknown format | ||
| null | ||
| } else { | ||
| (instant.dataType, instant.eval(input)) match { | ||
| case (_: DateType, date: Int) => | ||
| DateTimeUtils.truncateInstant(date, level) | ||
| case (_: TimestampType, timestamp: Long) => | ||
| DateTimeUtils.truncateInstant(timestamp, level) | ||
| case (_, null) => null | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -964,13 +985,13 @@ case class TruncDate(date: Expression, format: Expression) | |
| boolean ${ev.isNull} = true; | ||
| ${ctx.javaType(dataType)} ${ev.value} = ${ctx.defaultValue(dataType)};""") | ||
| } else { | ||
| val ist = instant.genCode(ctx) | ||
| ev.copy(code = s""" | ||
| ${ist.code} | ||
| boolean ${ev.isNull} = ${ist.isNull}; | ||
| ${ctx.javaType(dataType)} ${ev.value} = ${ctx.defaultValue(dataType)}; | ||
| if (!${ev.isNull}) { | ||
| ${ev.value} = $dtu.truncateInstant(${ist.value}, $truncLevel); | ||
| }""") | ||
| } | ||
| } else { | ||
|
|
@@ -981,7 +1002,7 @@ case class TruncDate(date: Expression, format: Expression) | |
| if ($form == -1) { | ||
| ${ev.isNull} = true; | ||
| } else { | ||
| ${ev.value} = $dtu.truncateInstant($dateVal, $form); | ||
| } | ||
| """ | ||
| }) | ||
|
|
||
This file contains hidden or 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
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may understand why hive call it
date_add...Other databases also have
date_add, but it takes interval type, which means it can handle days or months or both.To keep hive-compatibility, we should keep it as it was, but rename the expression class name LGTM