MaxGekk commented on a change in pull request #32893:
URL: https://github.com/apache/spark/pull/32893#discussion_r650556913
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2514,8 +2514,19 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with
SQLConfHelper with Logg
}
override def visitDayTimeIntervalDataType(ctx:
DayTimeIntervalDataTypeContext): DataType = {
- // TODO(SPARK-35736): Parse any day-time interval types in SQL
- DayTimeIntervalType()
+ def strToFieldIndex(str: String): Byte = str match {
Review comment:
You could avoid hardcoded constants:
```scala
val strToFieldIndex = dayTimeFields.map(i => fieldToString(i) -> i).toMap
```
##########
File path:
sql/catalyst/src/test/scala/org/apache/spark/sql/types/DataTypeSuite.scala
##########
@@ -258,6 +258,15 @@ class DataTypeSuite extends SparkFunSuite {
checkDataTypeFromDDL(YearMonthIntervalType)
checkDataTypeFromDDL(DayTimeIntervalType())
+ checkDataTypeFromDDL(DayTimeIntervalType(DayTimeIntervalType.DAY,
DayTimeIntervalType.MINUTE))
+ checkDataTypeFromDDL(DayTimeIntervalType(DayTimeIntervalType.DAY,
DayTimeIntervalType.HOUR))
+ checkDataTypeFromDDL(DayTimeIntervalType(DayTimeIntervalType.DAY,
DayTimeIntervalType.DAY))
+ checkDataTypeFromDDL(DayTimeIntervalType(DayTimeIntervalType.HOUR,
DayTimeIntervalType.SECOND))
+ checkDataTypeFromDDL(DayTimeIntervalType(DayTimeIntervalType.HOUR,
DayTimeIntervalType.MINUTE))
+ checkDataTypeFromDDL(DayTimeIntervalType(DayTimeIntervalType.HOUR,
DayTimeIntervalType.HOUR))
+ checkDataTypeFromDDL(DayTimeIntervalType(DayTimeIntervalType.MINUTE,
DayTimeIntervalType.SECOND))
+ checkDataTypeFromDDL(DayTimeIntervalType(DayTimeIntervalType.MINUTE,
DayTimeIntervalType.MINUTE))
+ checkDataTypeFromDDL(DayTimeIntervalType(DayTimeIntervalType.SECOND,
DayTimeIntervalType.SECOND))
Review comment:
Please, replace:
```suggestion
dayTimeIntervalTypes.foreach(checkDataTypeFromDDL)
```
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2514,8 +2514,19 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with
SQLConfHelper with Logg
}
override def visitDayTimeIntervalDataType(ctx:
DayTimeIntervalDataTypeContext): DataType = {
- // TODO(SPARK-35736): Parse any day-time interval types in SQL
- DayTimeIntervalType()
+ def strToFieldIndex(str: String): Byte = str match {
+ case "day" => 0
Review comment:
Could you replace the magic numbers by the constants from:
https://github.com/apache/spark/blob/d53831ff5cef618cfbc08dcea84ca9bb61ed9903/sql/catalyst/src/main/scala/org/apache/spark/sql/types/DayTimeIntervalType.scala#L85-L88
##########
File path:
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
##########
@@ -2514,8 +2514,19 @@ class AstBuilder extends SqlBaseBaseVisitor[AnyRef] with
SQLConfHelper with Logg
}
override def visitDayTimeIntervalDataType(ctx:
DayTimeIntervalDataTypeContext): DataType = {
- // TODO(SPARK-35736): Parse any day-time interval types in SQL
- DayTimeIntervalType()
+ def strToFieldIndex(str: String): Byte = str match {
+ case "day" => 0
+ case "hour" => 1
+ case "minute" => 2
+ case "second" => 3
+ }
+ val from = strToFieldIndex(ctx.from.getText.toLowerCase(Locale.ROOT))
+ val to = if (ctx.to != null ) {
+ strToFieldIndex(ctx.to.getText.toLowerCase(Locale.ROOT))
+ } else {
+ from
+ }
+ DayTimeIntervalType(from, to)
Review comment:
Could you use `start` and `end` as in the SQL standard.
--
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:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
