dybyte commented on code in PR #10360:
URL: https://github.com/apache/seatunnel/pull/10360#discussion_r2715681320


##########
docs/en/introduction/concepts/incompatible-changes.md:
##########
@@ -13,6 +13,14 @@ You need to check this document before you upgrade to 
related version.
 
 ### Transform Changes
 
+- **[BREAKING]** SQL Transform `PARSEDATETIME` and `TO_DATE` functions now 
only accept whitelisted datetime format patterns. Custom format patterns that 
were previously accepted will now fail at runtime. The supported patterns are:
+  - DateTime: `yyyy-MM-dd HH:mm:ss`, `yyyy-MM-dd HH:mm:ss.SSS`, 
`yyyy-MM-dd'T'HH:mm:ss`, `yyyy-MM-dd'T'HH:mm:ss.SSS`
+  - Date: `yyyy-MM-dd`
+  - Time: `HH:mm:ss`, `HH:mm:ss.SSS`
+
+
+  **Migration Guide**: If you are using custom datetime format patterns in 
`PARSEDATETIME` or `TO_DATE` functions, you must update your queries to use one 
of the supported patterns above. If your data uses a different format, you may 
need to preprocess the input data to match a supported format, or use string 
manipulation functions to transform the format before parsing.

Review Comment:
   Since the exception for invalid formats has changed from 
`TransformException` to `SeaTunnelRuntimeException`, it would be great to 
mention this change in the compatibility documentation. This will help users 
update their error handling or monitoring systems accordingly.



##########
seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/sql/zeta/functions/DateTimeFunctionsTest.java:
##########
@@ -319,11 +319,127 @@ public void testDateAddWithUnsupportedField() {
                         new SeaTunnelDataType[] 
{LocalTimeType.LOCAL_DATE_TYPE});
 
         Assertions.assertThrows(
-                TransformException.class,
+                SeaTunnelRuntimeException.class,
                 () ->
                         runSql(
                                 "select DATEADD(dt, 1, 'UNSUPPORTED') as d 
from dual",
                                 rowType,
                                 LocalDate.of(2024, 6, 15)));

Review Comment:
   I don’t think this test is affected by the current PR. Is it related?



##########
seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/sql/zeta/ZetaSQLTypeTest.java:
##########
@@ -355,14 +356,14 @@ public void testCoalesceMultiIfModAndUdfTypes() {
         Function multiIfNoParams = new Function();
         multiIfNoParams.setName(ZetaSQLFunction.MULTI_IF);
         Assertions.assertThrows(
-                TransformException.class, () -> 
type.getExpressionType(multiIfNoParams));
+                SeaTunnelRuntimeException.class, () -> 
type.getExpressionType(multiIfNoParams));
 
         Function multiIfEvenArgs = new Function();
         multiIfEvenArgs.setName(ZetaSQLFunction.MULTI_IF);
         multiIfEvenArgs.setParameters(
                 new ExpressionList<>(Arrays.asList(new LongValue(1), new 
LongValue(1))));
         Assertions.assertThrows(
-                TransformException.class, () -> 
type.getExpressionType(multiIfEvenArgs));
+                SeaTunnelRuntimeException.class, () -> 
type.getExpressionType(multiIfEvenArgs));

Review Comment:
   ditto



##########
seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/sql/zeta/ZetaSQLTypeTest.java:
##########
@@ -437,6 +438,6 @@ public void testGetMaxTypeCollection() {
         Assertions.assertEquals(BasicType.DOUBLE_TYPE, result);
 
         Assertions.assertThrows(
-                TransformException.class, () -> 
type.getMaxType(Collections.emptyList()));
+                SeaTunnelRuntimeException.class, () -> 
type.getMaxType(Collections.emptyList()));

Review Comment:
   ditto



##########
seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/sql/zeta/ZetaSQLTypeTest.java:
##########
@@ -388,7 +389,7 @@ public void testCoalesceMultiIfModAndUdfTypes() {
         unknownFunc.setParameters(
                 new ExpressionList<>(Collections.singletonList(new 
LongValue(1))));
         Assertions.assertThrows(
-                TransformException.class, () -> 
udfType.getExpressionType(unknownFunc));
+                SeaTunnelRuntimeException.class, () -> 
udfType.getExpressionType(unknownFunc));

Review Comment:
   ditto



##########
seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/sql/zeta/ZetaSQLTypeTest.java:
##########
@@ -338,7 +339,7 @@ public void testCoalesceMultiIfModAndUdfTypes() {
         Function badCoalesce = new Function();
         badCoalesce.setName(ZetaSQLFunction.COALESCE);
         Assertions.assertThrows(
-                TransformException.class, () -> 
type.getExpressionType(badCoalesce));
+                SeaTunnelRuntimeException.class, () -> 
type.getExpressionType(badCoalesce));

Review Comment:
   ditto



##########
seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/sql/zeta/ZetaSQLTypeTest.java:
##########
@@ -420,7 +421,7 @@ public void testIsNumberTypeAndGetMaxType() {
         Assertions.assertEquals(intType, type.getMaxType(intType, null));
 
         Assertions.assertThrows(
-                TransformException.class,
+                SeaTunnelRuntimeException.class,

Review Comment:
   ditto



##########
seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/sql/zeta/functions/DateTimeFunctionsTest.java:
##########
@@ -319,11 +319,127 @@ public void testDateAddWithUnsupportedField() {
                         new SeaTunnelDataType[] 
{LocalTimeType.LOCAL_DATE_TYPE});
 
         Assertions.assertThrows(
-                TransformException.class,
+                SeaTunnelRuntimeException.class,
                 () ->
                         runSql(
                                 "select DATEADD(dt, 1, 'UNSUPPORTED') as d 
from dual",
                                 rowType,
                                 LocalDate.of(2024, 6, 15)));
     }
+
+    @Test
+    public void testParseDateTimeWithAllDateTimeFormats() {
+        SeaTunnelRowType rowType =
+                new SeaTunnelRowType(
+                        new String[] {"dummy"},
+                        new SeaTunnelDataType[] 
{LocalTimeType.LOCAL_DATE_TIME_TYPE});
+
+        // DATETIME_STANDARD: yyyy-MM-dd HH:mm:ss
+        SeaTunnelRow row1 =
+                runSql(
+                        "select PARSEDATETIME('2024-06-15 14:30:45', 
'yyyy-MM-dd HH:mm:ss') as dt from dual",
+                        rowType,
+                        LocalDateTime.now());
+        Assertions.assertEquals(LocalDateTime.of(2024, 6, 15, 14, 30, 45), 
row1.getField(0));
+
+        // DATETIME_WITH_MILLIS: yyyy-MM-dd HH:mm:ss.SSS
+        SeaTunnelRow row2 =
+                runSql(
+                        "select PARSEDATETIME('2024-06-15 14:30:45.123', 
'yyyy-MM-dd HH:mm:ss.SSS') as dt from dual",
+                        rowType,
+                        LocalDateTime.now());
+        Assertions.assertEquals(
+                LocalDateTime.of(2024, 6, 15, 14, 30, 45, 123000000), 
row2.getField(0));
+
+        // DATETIME_ISO8601: yyyy-MM-dd'T'HH:mm:ss
+        SeaTunnelRow row3 =
+                runSql(
+                        "select PARSEDATETIME('2024-06-15T14:30:45', 
'yyyy-MM-dd''T''HH:mm:ss') as dt from dual",
+                        rowType,
+                        LocalDateTime.now());
+        Assertions.assertEquals(LocalDateTime.of(2024, 6, 15, 14, 30, 45), 
row3.getField(0));
+
+        // DATETIME_ISO8601_WITH_MILLIS: yyyy-MM-dd'T'HH:mm:ss.SSS
+        SeaTunnelRow row4 =
+                runSql(
+                        "select PARSEDATETIME('2024-06-15T14:30:45.987', 
'yyyy-MM-dd''T''HH:mm:ss.SSS') as dt from dual",
+                        rowType,
+                        LocalDateTime.now());
+        Assertions.assertEquals(
+                LocalDateTime.of(2024, 6, 15, 14, 30, 45, 987000000), 
row4.getField(0));
+    }
+
+    @Test
+    public void testParseDateTimeWithAllTimeFormats() {
+        SeaTunnelRowType rowType =
+                new SeaTunnelRowType(
+                        new String[] {"dummy"},
+                        new SeaTunnelDataType[] 
{LocalTimeType.LOCAL_DATE_TIME_TYPE});
+
+        // TIME_STANDARD: HH:mm:ss
+        SeaTunnelRow row1 =
+                runSql(
+                        "select PARSEDATETIME('14:30:45', 'HH:mm:ss') as dt 
from dual",
+                        rowType,
+                        LocalDateTime.now());
+        Assertions.assertEquals(java.time.LocalTime.of(14, 30, 45), 
row1.getField(0));
+
+        // TIME_WITH_MILLIS: HH:mm:ss.SSS
+        SeaTunnelRow row2 =
+                runSql(
+                        "select PARSEDATETIME('14:30:45.123', 'HH:mm:ss.SSS') 
as dt from dual",
+                        rowType,
+                        LocalDateTime.now());
+        Assertions.assertEquals(java.time.LocalTime.of(14, 30, 45, 123000000), 
row2.getField(0));
+    }
+
+    @Test
+    public void testParseDateTimeWithUnsupportedFormat() {
+        SeaTunnelRowType rowType =
+                new SeaTunnelRowType(
+                        new String[] {"dummy"},
+                        new SeaTunnelDataType[] 
{LocalTimeType.LOCAL_DATE_TIME_TYPE});
+
+        // Test with completely unsupported format
+        Assertions.assertThrows(
+                SeaTunnelRuntimeException.class,
+                () ->
+                        runSql(
+                                "select PARSEDATETIME('2024-06-15', 
'dd/MM/yyyy') as dt from dual",
+                                rowType,
+                                LocalDateTime.now()));
+    }
+
+    @Test
+    public void testParseDateTimeWithMalformedInput() {
+        SeaTunnelRowType rowType =
+                new SeaTunnelRowType(
+                        new String[] {"dummy"},
+                        new SeaTunnelDataType[] 
{LocalTimeType.LOCAL_DATE_TIME_TYPE});
+
+        // Test with malformed input string
+        Assertions.assertThrows(
+                SeaTunnelRuntimeException.class,
+                () ->
+                        runSql(
+                                "select PARSEDATETIME('not-a-date', 
'yyyy-MM-dd') as dt from dual",
+                                rowType,
+                                LocalDateTime.now()));
+    }
+
+    @Test
+    public void testToDateWithVariousFormats() {

Review Comment:
   ```suggestion
       public void testToDateWithDateFormat() {
   ```



##########
seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/sql/zeta/ZetaSQLTypeTest.java:
##########
@@ -355,14 +356,14 @@ public void testCoalesceMultiIfModAndUdfTypes() {
         Function multiIfNoParams = new Function();
         multiIfNoParams.setName(ZetaSQLFunction.MULTI_IF);
         Assertions.assertThrows(
-                TransformException.class, () -> 
type.getExpressionType(multiIfNoParams));
+                SeaTunnelRuntimeException.class, () -> 
type.getExpressionType(multiIfNoParams));

Review Comment:
   ditto



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

Reply via email to