abhinav-phi commented on code in PR #68732:
URL: https://github.com/apache/doris/pull/68732#discussion_r4296947279


##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/DateTimeExtractAndTransform.java:
##########
@@ -408,7 +408,11 @@ public static Expression timeFormat(TimeV2Literal time, 
StringLikeLiteral format
             throw new AnalysisException("The length of format string in 
time_format() function should not be greater"
                     + " than 128.");
         }
-        return new 
VarcharLiteral(DateTimeFormatterUtils.toFormatStringConservative(time, format));
+        String formatted = 
DateTimeFormatterUtils.toFormatStringConservative(time, format);
+        if (formatted == null) {
+            return new NullLiteral(StringType.INSTANCE);

Review Comment:
   fixed in 80577b45. the folded NULL now carries the declared return type 
instead of STRING:
   
       return new NullLiteral(VarcharType.SYSTEM_DEFAULT);
   
   both sides of the case are asserted in DateTimeExtractAndTransformTest, the 
negative time and the positive one, each checking `getDataType()` on the 
NullLiteral comes back as `VarcharType.SYSTEM_DEFAULT`, so a `SELECT 
time_format(...) AS fmt FROM table` advertises VARCHAR to the client rather 
than STRING/BLOB. the time specifier path is asserted in the same test so the 
sign of a negative time is still kept.



-- 
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]

Reply via email to