davidm-db commented on code in PR #53344:
URL: https://github.com/apache/spark/pull/53344#discussion_r2603312828
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -3499,7 +3499,7 @@ class AstBuilder extends DataTypeAstBuilder
val zoneId = getZoneId(conf.sessionLocalTimeZone)
val specialDate = convertSpecialDate(value, zoneId).map(Literal(_,
DateType))
specialDate.getOrElse(toLiteral(stringToDate, DateType))
- case TIME => toLiteral(stringToTime, TimeType())
+ case TIME if conf.isTimeTypeEnabled => toLiteral(stringToTime,
TimeType())
Review Comment:
I replicated what Max did internally. I think the reason for this is:
- the code you are commenting is handling literal types (statement example:
`SELECT TIME'10:00:00'`) and is done this way to fit into the existing error
message format
- `{time_type_enabled}?` guard in `SqlBaseParser.g4` guards references to
the TIME as a type and throws different class of errors, i.e. datatype
unsupported (statement example: `CREATE TABLE t(col TIME)`)
I don't know if we want to change this behavior or not, feel free to share
your thoughts.
##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala:
##########
@@ -3499,7 +3499,7 @@ class AstBuilder extends DataTypeAstBuilder
val zoneId = getZoneId(conf.sessionLocalTimeZone)
val specialDate = convertSpecialDate(value, zoneId).map(Literal(_,
DateType))
specialDate.getOrElse(toLiteral(stringToDate, DateType))
- case TIME => toLiteral(stringToTime, TimeType())
+ case TIME if conf.isTimeTypeEnabled => toLiteral(stringToTime,
TimeType())
Review Comment:
I replicated what Max did internally. I think the reason for this is:
- the code you are commenting is handling literal types (statement example:
`SELECT TIME'10:00:00'`) and is done this way to fit into the existing error
message format
- `{time_type_enabled}?` guard in `SqlBaseParser.g4` guards references to
the TIME as a type and throws different class of errors, i.e. datatype
unsupported (statement example: `CREATE TABLE t(col TIME)`)
I don't know if we want to change this behavior or not, please share your
thoughts.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
