dybyte commented on code in PR #10390:
URL: https://github.com/apache/seatunnel/pull/10390#discussion_r2749416345
##########
seatunnel-formats/seatunnel-format-json/src/main/java/org/apache/seatunnel/format/json/JsonToRowConverters.java:
##########
@@ -258,16 +258,29 @@ private float convertToFloat(JsonNode jsonNode) {
private LocalDate convertToLocalDate(JsonNode jsonNode, String fieldName) {
String dateStr = jsonNode.asText();
- DateTimeFormatter dateFormatter = fieldFormatterMap.get(fieldName);
+
+ if (dateStr == null || dateStr.trim().isEmpty()) {
+ return null;
+ }
Review Comment:
Could you clarify the intent of this if statement? It seems to allow empty
strings to pass as null, which breaks the existing validation logic. I would
suggest removing it to ensure that invalid date strings still trigger the
appropriate exception.
##########
seatunnel-formats/seatunnel-format-json/src/main/java/org/apache/seatunnel/format/json/JsonToRowConverters.java:
##########
@@ -277,11 +290,24 @@ private LocalTime convertToLocalTime(JsonNode jsonNode) {
private LocalDateTime convertToLocalDateTime(JsonNode jsonNode, String
fieldName) {
String datetimeStr = jsonNode.asText();
- DateTimeFormatter dateTimeFormatter = fieldFormatterMap.get(fieldName);
+
+ if (datetimeStr == null || datetimeStr.trim().isEmpty()) {
+ return null;
+ }
Review Comment:
ditto
##########
seatunnel-formats/seatunnel-format-json/src/test/java/org/apache/seatunnel/format/json/JsonRowDataSerDeSchemaTest.java:
##########
@@ -713,4 +713,54 @@ public void testSerializationWithNumber() {
String expected = "{\"id\":1,\"code\":\"1001015\",\"fe_result\":80}";
assertEquals(new String(serialize), expected);
}
+
+ @Test
+ public void testConvertToLocalDateWithEmptyString() throws IOException {
+ SeaTunnelRowType rowType =
+ new SeaTunnelRowType(
+ new String[] {"date_field"},
+ new SeaTunnelDataType<?>[]
{LocalTimeType.LOCAL_DATE_TYPE});
+ JsonDeserializationSchema deserializationSchema =
+ new JsonDeserializationSchema(false, false, rowType);
+
+ // Test empty string
+ String emptyDateJson = "{\"date_field\":\"\"}";
+ SeaTunnelRow row =
deserializationSchema.deserialize(emptyDateJson.getBytes());
+ assertNull(row.getField(0));
+
+ // Test whitespace only string
+ String whitespaceJson = "{\"date_field\":\" \"}";
+ row = deserializationSchema.deserialize(whitespaceJson.getBytes());
+ assertNull(row.getField(0));
+
+ // Test normal date value still works
+ String normalDateJson = "{\"date_field\":\"2024-01-15\"}";
+ row = deserializationSchema.deserialize(normalDateJson.getBytes());
+ assertEquals(LocalDate.of(2024, 1, 15), row.getField(0));
+ }
+
+ @Test
+ public void testConvertToLocalDateTimeWithEmptyString() throws IOException
{
+ SeaTunnelRowType rowType =
+ new SeaTunnelRowType(
+ new String[] {"timestamp_field"},
+ new SeaTunnelDataType<?>[]
{LocalTimeType.LOCAL_DATE_TIME_TYPE});
+ JsonDeserializationSchema deserializationSchema =
+ new JsonDeserializationSchema(false, false, rowType);
+
+ // Test empty string
+ String emptyTimestampJson = "{\"timestamp_field\":\"\"}";
+ SeaTunnelRow row =
deserializationSchema.deserialize(emptyTimestampJson.getBytes());
+ assertNull(row.getField(0));
+
+ // Test whitespace only string
+ String whitespaceJson = "{\"timestamp_field\":\" \"}";
+ row = deserializationSchema.deserialize(whitespaceJson.getBytes());
+ assertNull(row.getField(0));
+
+ // Test normal timestamp value still works
+ String normalTimestampJson = "{\"timestamp_field\":\"2024-01-15
10:30:00\"}";
+ row =
deserializationSchema.deserialize(normalTimestampJson.getBytes());
+ assertEquals(LocalDateTime.of(2024, 1, 15, 10, 30, 0),
row.getField(0));
+ }
Review Comment:
Could you please add a test case related to this issue in
`JsonRowDataSerDeSchemaTest.java`? It would be great to ensure this scenario is
covered and to prevent any future regressions.
--
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]
