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