kurnoolsaketh commented on code in PR #29493:
URL: https://github.com/apache/flink/pull/29493#discussion_r4486752752
##########
flink-table/flink-table-planner/src/main/scala/org/apache/flink/table/planner/codegen/JsonGenerateUtils.scala:
##########
@@ -92,6 +92,18 @@ object JsonGenerateUtils {
case TIMESTAMP_WITH_LOCAL_TIME_ZONE =>
s"""$nodeFactoryTerm.textNode($isoTerm + "Z")"""
}
+ case TIME_WITHOUT_TIME_ZONE =>
+ val formatter =
s"${typeTerm(classOf[DateTimeFormatter])}.ISO_LOCAL_TIME"
+ val isoTerm =
+
s"${typeTerm(classOf[DateTimeUtils])}.toLocalTime($valueTerm).format($formatter)"
+ s"$nodeFactoryTerm.textNode($isoTerm)"
+
+ case DATE =>
+ val formatter =
s"${typeTerm(classOf[DateTimeFormatter])}.ISO_LOCAL_DATE"
+ val isoTerm =
+
s"${typeTerm(classOf[DateTimeUtils])}.toLocalDate($valueTerm).format($formatter)"
+ s"$nodeFactoryTerm.textNode($isoTerm)"
Review Comment:
> can we reuse DateTimeUtils
if I am understanding you correctly, we _are_ using DateTimeUtils here
either way - we just have a choice on what helper function to use. See below
for my opinion on that.
> for instance DateTimeUtils.formatDate
we can reuse it, but it doesn't handle edge cases well. For ex, overflow
cases: `DateTimeUtils.formatDate(Integer.MAX_VALUE)` outputs `)*,0-0+-0(`
whereas `DateTimeUtils.toLocalDate(Integer.MAX_VALUE).format(ISO_LOCAL_DATE)`
throws since it's an invalid date. So, I prefer to use `toLocalDate` and
`format` since it is uses java std libraries.
--
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]
