You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/04/21 10:36:45 UTC
[GitHub] [spark] yaooqinn opened a new pull request #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, m…
yaooqinn opened a new pull request #28284:
URL: https://github.com/apache/spark/pull/28284
…icrosecond and epoch fields support from extract fucntion
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
Extracting millennium, century, decade, millisecond, microsecond and epoch from datetime is neither ANSI standard nor quite common in modern SQL platforms. None of the systems listing below support these except PostgreSQL.
https://cwiki.apache.org/confluence/display/Hive/LanguageManual+UDF
https://docs.oracle.com/cd/B19306_01/server.102/b14200/functions050.htm
https://prestodb.io/docs/current/functions/datetime.html
https://docs.cloudera.com/documentation/enterprise/5-8-x/topics/impala_datetime_functions.html
https://docs.snowflake.com/en/sql-reference/functions-date-time.html#label-supported-date-time-parts
https://www.postgresql.org/docs/9.1/functions-datetime.html#FUNCTIONS-DATETIME-EXTRACT
This PR remove these extract fields support from extract function for date and timestamp values
### Why are the changes needed?
Remove some nonstandard and uncommon features
### Does this PR introduce any user-facing change?
<!--
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If no, write 'No'.
-->
NO, we should target this to 3.0.0 and these are added during 3.0.0
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Remove unused tests
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412638011
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
@yaooqinn . Ya. It's clear for that part. Could you enumerate the name of systems which supports `DAYOFWEEK_ISO`? (except IBM DB i). I'm wondering that specifically.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617182503
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617191959
cc @MaxGekk @dongjoon-hyun @HyukjinKwon
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, microsecond and epoch fields support from extract function
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617181570
**[Test build #121586 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121586/testReport)** for PR 28284 at commit [`521bc2c`](https://github.com/apache/spark/commit/521bc2cd45b21d01bc1e0d374f913406d1afeb42).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412648326
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Specifically, snowflake use dayofweek_iso except db2. I couldn't find more because most platforms do not have two week-numbering system implemented.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617182503
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412379663
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Just a question. This PR is adding `DAYOFWEEK_ISO` newly. Which platform are you referring for `DAYOFWEEK_ISO`? IIRC, @gatorsmile commented that we should not consider IBM DB i.
> isodow is PostgreSQL specific but iso as a suffix is more commonly used across platforms
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412638011
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
@yaooqinn . I asked the explanation about your PR description.
> isodow is PostgreSQL specific but iso as a suffix is more commonly used across platforms
Let me ask again specifically? Could you enumerate the name of systems which supports `DAYOFWEEK_ISO`? (except IBM DB i).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617691281
thanks, merging to master/3.0!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412621357
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Hi @dongjoon-hyun, for historical reasons, we have [`dayofweek`, `dow`] implemented for representing a non-ISO week of week-based-year and a newly added `isodow` from PostgreSQL for ISO week of week-based-year. Many other systems only have one week-numbering system support and use either full names or abbreviations. Things in spark become a little bit complicated.
1. because of the existence of `isodow`, so we need to add iso-prefix to `dayofweek` to make a pair for it too. [`dayofweek`, `isodayofweek`, `dow` and `isodow`]
2. because there are rare `iso`-prefixed systems and more systems choose `iso`-suffixed way, so we may result in [`dayofweek`, `dayofweekiso`, `dow`, `dowiso`]
3. `dayofweekiso` looks nice and has use cases in the platforms listed above, e.g. snowflake, but `dowiso` looks weird and no use cases found.
4. after a discussion with @cloud-fan, we have both agreed with an underscore before `iso` may look much better because `isodow` is new and there is no standard for `iso` kind of things, so this may be good for us to make it simple and clear for end-users
Thus, we finally result in [`dayofweek`, `dow`] for Non-ISO week-numbering system and [`dayofweek_iso`, `dow_iso`] for ISO system
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, m…
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617098461
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412638011
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
@yaooqinn . Ya. It's clear for that part. But, I asked the explanation about your PR description.
> isodow is PostgreSQL specific but iso as a suffix is more commonly used across platforms
Let me ask again specifically. Could you enumerate the name of systems which supports `DAYOFWEEK_ISO`? (except IBM DB i). I'm wondering that.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, m…
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617139425
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617181570
**[Test build #121586 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121586/testReport)** for PR 28284 at commit [`521bc2c`](https://github.com/apache/spark/commit/521bc2cd45b21d01bc1e0d374f913406d1afeb42).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, m…
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617139437
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/121581/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412621357
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Hi @dongjoon-hyun, for historical reasons, we have [`dayofweek`, `dow` implemented for representing a non-ISO week of week-based-year and a newly added `isodow` for ISO week of week-based-year. Many other systems do not only have one week-numbering system support. Things in spark become a little bit complicated.
1. because of the existence of `isodow`, so we need to add iso-prefix to `dayofweek` to make a pair for it too. [`dayofweek`, `isodayofweek`, `dow` and `isodow`]
2. because there are rare `iso`-prefixed systems and more systems choose `iso`-suffixed way, so we may result in [`dayofweek`, `dayofweekiso`, `dow`, `dowiso`]
3. `dayofweekiso` looks nice and has use cases in the platforms listed above, e.g. snowflake, but `dowiso` looks weird and no use cases found.
4. after a discussion with @cloud-fan, we have both agreed with an underscore before `iso` may look much better because `isodow` is new and there is no standard for `iso` kind of things, so this may be good for us to make it simple and clear for end-users
Thus, we finally result in [`dayofweek`, `dow`] for Non-ISO week-numbering system and [`dayofweek_iso`, `dow_iso`] for ISO system
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412621357
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Hi @dongjoon-hyun, for historical reasons, we have [`dayofweek`, `dow`] implemented for representing a non-ISO week of week-based-year and a newly added `isodow` from PostgreSQL for ISO week of week-based-year. Many other systems only have one week-numbering system support. Things in spark become a little bit complicated.
1. because of the existence of `isodow`, so we need to add iso-prefix to `dayofweek` to make a pair for it too. [`dayofweek`, `isodayofweek`, `dow` and `isodow`]
2. because there are rare `iso`-prefixed systems and more systems choose `iso`-suffixed way, so we may result in [`dayofweek`, `dayofweekiso`, `dow`, `dowiso`]
3. `dayofweekiso` looks nice and has use cases in the platforms listed above, e.g. snowflake, but `dowiso` looks weird and no use cases found.
4. after a discussion with @cloud-fan, we have both agreed with an underscore before `iso` may look much better because `isodow` is new and there is no standard for `iso` kind of things, so this may be good for us to make it simple and clear for end-users
Thus, we finally result in [`dayofweek`, `dow`] for Non-ISO week-numbering system and [`dayofweek_iso`, `dow_iso`] for ISO system
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412621357
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Hi @dongjoon-hyun, for historical reasons, we have [`dayofweek`, `dow`] implemented for representing a non-ISO day-of-week and a newly added `isodow` from PostgreSQL for ISO day-of-week. Many other systems only have one week-numbering system support and use either full names or abbreviations. Things in spark become a little bit complicated.
1. because of the existence of `isodow`, so we need to add iso-prefix to `dayofweek` to make a pair for it too. [`dayofweek`, `isodayofweek`, `dow` and `isodow`]
2. because there are rare `iso`-prefixed systems and more systems choose `iso`-suffixed way, so we may result in [`dayofweek`, `dayofweekiso`, `dow`, `dowiso`]
3. `dayofweekiso` looks nice and has use cases in the platforms listed above, e.g. snowflake, but `dowiso` looks weird and no use cases found.
4. after a discussion with @cloud-fan, we have both agreed with an underscore before `iso` may look much better because `isodow` is new and there is no standard for `iso` kind of things, so this may be good for us to make it simple and clear for end-users if they are well documented too.
Thus, we finally result in [`dayofweek`, `dow`] for Non-ISO week-numbering system and [`dayofweek_iso`, `dow_iso`] for ISO system
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412621357
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Hi @dongjoon-hyun, for historical reasons, we have [`dayofweek`, `dow`] implemented for representing a non-ISO week of week-based-year and a newly added `isodow` from PostgreSQL for ISO week of week-based-year. Many other systems do not only have one week-numbering system support. Things in spark become a little bit complicated.
1. because of the existence of `isodow`, so we need to add iso-prefix to `dayofweek` to make a pair for it too. [`dayofweek`, `isodayofweek`, `dow` and `isodow`]
2. because there are rare `iso`-prefixed systems and more systems choose `iso`-suffixed way, so we may result in [`dayofweek`, `dayofweekiso`, `dow`, `dowiso`]
3. `dayofweekiso` looks nice and has use cases in the platforms listed above, e.g. snowflake, but `dowiso` looks weird and no use cases found.
4. after a discussion with @cloud-fan, we have both agreed with an underscore before `iso` may look much better because `isodow` is new and there is no standard for `iso` kind of things, so this may be good for us to make it simple and clear for end-users
Thus, we finally result in [`dayofweek`, `dow`] for Non-ISO week-numbering system and [`dayofweek_iso`, `dow_iso`] for ISO system
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412621357
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Hi @dongjoon-hyun, for historical reasons, we have [`dayofweek`, `dow`] implemented for representing a non-ISO week of week-based-year and a newly added `isodow` for ISO week of week-based-year. Many other systems do not only have one week-numbering system support. Things in spark become a little bit complicated.
1. because of the existence of `isodow`, so we need to add iso-prefix to `dayofweek` to make a pair for it too. [`dayofweek`, `isodayofweek`, `dow` and `isodow`]
2. because there are rare `iso`-prefixed systems and more systems choose `iso`-suffixed way, so we may result in [`dayofweek`, `dayofweekiso`, `dow`, `dowiso`]
3. `dayofweekiso` looks nice and has use cases in the platforms listed above, e.g. snowflake, but `dowiso` looks weird and no use cases found.
4. after a discussion with @cloud-fan, we have both agreed with an underscore before `iso` may look much better because `isodow` is new and there is no standard for `iso` kind of things, so this may be good for us to make it simple and clear for end-users
Thus, we finally result in [`dayofweek`, `dow`] for Non-ISO week-numbering system and [`dayofweek_iso`, `dow_iso`] for ISO system
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412379663
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Just a question. Which platform are you referring for `DAYOFWEEK_ISO`? IIRC, @gatorsmile commented that we should not consider IBM DB i.
> isodow is PostgreSQL specific but iso as a suffix is more commonly used across platforms
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412650911
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Got it. Thanks. I understand that Snowflake is considered important. Could you add some of the above comment(https://github.com/apache/spark/pull/28284#discussion_r412621357) to the PR description then?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617877752
Thank you all!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412638011
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
@yaooqinn . Ya. It's clear for that part. But, I asked the explanation about your PR description.
> isodow is PostgreSQL specific but iso as a suffix is more commonly used across platforms
Let me ask again specifically? Could you enumerate the name of systems which supports `DAYOFWEEK_ISO`? (except IBM DB i).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617324171
cc @maropu
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412379663
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Unlike the PR title, this PR is adding `DAYOFWEEK_ISO` newly. Which platform are you referring for `DAYOFWEEK_ISO`? IIRC, @gatorsmile commented that we should not consider IBM DB i.
> isodow is PostgreSQL specific but iso as a suffix is more commonly used across platforms
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on issue #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, m…
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617101423
**[Test build #121581 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121581/testReport)** for PR 28284 at commit [`3146b65`](https://github.com/apache/spark/commit/3146b657f8310804dfa5bcc7fb83f5f35b1db8be).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412621357
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Hi @dongjoon-hyun, for historical reasons, we have [`dayofweek`, `dow`] implemented for representing a non-ISO week of week-based-year and a newly added `isodow` from PostgreSQL for ISO week of week-based-year. Many other systems only have one week-numbering system support and use either full names or abbreviations. Things in spark become a little bit complicated.
1. because of the existence of `isodow`, so we need to add iso-prefix to `dayofweek` to make a pair for it too. [`dayofweek`, `isodayofweek`, `dow` and `isodow`]
2. because there are rare `iso`-prefixed systems and more systems choose `iso`-suffixed way, so we may result in [`dayofweek`, `dayofweekiso`, `dow`, `dowiso`]
3. `dayofweekiso` looks nice and has use cases in the platforms listed above, e.g. snowflake, but `dowiso` looks weird and no use cases found.
4. after a discussion with @cloud-fan, we have both agreed with an underscore before `iso` may look much better because `isodow` is new and there is no standard for `iso` kind of things, so this may be good for us to make it simple and clear for end-users if they are well documented too.
Thus, we finally result in [`dayofweek`, `dow`] for Non-ISO week-numbering system and [`dayofweek_iso`, `dow_iso`] for ISO system
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617319393
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, m…
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617098461
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, m…
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617101423
**[Test build #121581 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121581/testReport)** for PR 28284 at commit [`3146b65`](https://github.com/apache/spark/commit/3146b657f8310804dfa5bcc7fb83f5f35b1db8be).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412379663
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Just a question. Which platform are you referring for `DAYOFWEEK_ISO`?
> isodow is PostgreSQL specific but iso as a suffix is more commonly used across platforms
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412663151
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
thanks @dongjoon-hyun. PR description updated
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617319393
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, m…
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617139312
**[Test build #121581 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121581/testReport)** for PR 28284 at commit [`3146b65`](https://github.com/apache/spark/commit/3146b657f8310804dfa5bcc7fb83f5f35b1db8be).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] yaooqinn commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
yaooqinn commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412648326
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
Specifically, snowflake use dayofweek_iso except db2. I couldn't find more because most platforms do not have two day-of-week system implemented.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28284:
URL: https://github.com/apache/spark/pull/28284#discussion_r412638011
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/datetimeExpressions.scala
##########
@@ -2033,108 +1991,26 @@ case class MakeTimestamp(
override def prettyName: String = "make_timestamp"
}
-case class Millennium(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getMillennium(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getMillennium($c)")
- }
-}
-
-case class Century(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getCentury(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getCentury($c)")
- }
-}
-
-case class Decade(child: Expression) extends UnaryExpression with ImplicitCastInputTypes {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(DateType)
-
- override def dataType: DataType = IntegerType
-
- override protected def nullSafeEval(date: Any): Any = {
- DateTimeUtils.getDecade(date.asInstanceOf[Int])
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- defineCodeGen(ctx, ev, c => s"$dtu.getDecade($c)")
- }
-}
-
-case class Epoch(child: Expression, timeZoneId: Option[String] = None)
- extends UnaryExpression with ImplicitCastInputTypes with TimeZoneAwareExpression {
-
- override def inputTypes: Seq[AbstractDataType] = Seq(TimestampType)
- // DecimalType is used to not lose precision while converting microseconds to
- // the fractional part of seconds. Scale 6 is taken to have all microseconds as
- // the fraction. The precision 20 should cover whole valid range of years [1, 9999]
- // plus negative years that can be used in some cases though are not officially supported.
- override def dataType: DataType = DecimalType(20, 6)
- override def withTimeZone(timeZoneId: String): TimeZoneAwareExpression =
- copy(timeZoneId = Option(timeZoneId))
-
- override protected def nullSafeEval(timestamp: Any): Any = {
- DateTimeUtils.getEpoch(timestamp.asInstanceOf[Long], zoneId)
- }
-
- override protected def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
- val dtu = DateTimeUtils.getClass.getName.stripSuffix("$")
- val zid = ctx.addReferenceObj("zoneId", zoneId, classOf[ZoneId].getName)
- defineCodeGen(ctx, ev, c => s"$dtu.getEpoch($c, $zid)")
- }
-}
-
object DatePart {
def parseExtractField(
extractField: String,
source: Expression,
errorHandleFunc: => Nothing): Expression = extractField.toUpperCase(Locale.ROOT) match {
- case "MILLENNIUM" | "MILLENNIA" | "MIL" | "MILS" => Millennium(source)
- case "CENTURY" | "CENTURIES" | "C" | "CENT" => Century(source)
- case "DECADE" | "DECADES" | "DEC" | "DECS" => Decade(source)
case "YEAR" | "Y" | "YEARS" | "YR" | "YRS" => Year(source)
- case "ISOYEAR" => IsoYear(source)
+ case "YEAROFWEEK" => YearOfWeek(source)
case "QUARTER" | "QTR" => Quarter(source)
case "MONTH" | "MON" | "MONS" | "MONTHS" => Month(source)
case "WEEK" | "W" | "WEEKS" => WeekOfYear(source)
case "DAY" | "D" | "DAYS" => DayOfMonth(source)
case "DAYOFWEEK" | "DOW" => DayOfWeek(source)
- case "ISODOW" => Add(WeekDay(source), Literal(1))
+ case "DAYOFWEEK_ISO" | "DOW_ISO" => Add(WeekDay(source), Literal(1))
Review comment:
@yaooqinn . Ya. It's clear for that part. Could you enumerate the name of systems which supports `DAYOFWEEK_ISO`? (except IBM DB i). I'm wondering that specifically.
It's because you wrote like `Many other systems` in the PR description.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on issue #28284: [SPARK-31507][SQL] Remove millennium, century, decade, millisecond, m…
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617139425
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on issue #28284: [SPARK-31507][SQL] Remove uncommon fields support and update some fields with meaningful names for extract function
Posted by GitBox <gi...@apache.org>.
SparkQA commented on issue #28284:
URL: https://github.com/apache/spark/pull/28284#issuecomment-617318119
**[Test build #121586 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/121586/testReport)** for PR 28284 at commit [`521bc2c`](https://github.com/apache/spark/commit/521bc2cd45b21d01bc1e0d374f913406d1afeb42).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class YearOfWeek(child: Expression) extends UnaryExpression with ImplicitCastInputTypes `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org