[Fix][Transform-V2][JsonPath] Fix date/time conversion error when mul… #10390

yht0827 · 2026-01-23T15:55:59Z

Purpose of this pull request

When using JsonPath transform with multiple date/timestamp columns, the converter received null as the field name, causing incorrect date format resolution.
Fixed by passing columnConfig.getDestField() to the converter, ensuring each column resolves its own date format correctly.

Does this PR introduce any user-facing change?

Yes. Previously, multiple date/timestamp columns in JsonPath transform could fail or produce incorrect results due to missing field name context. Now each column correctly applies its configured date/time format.

How was this patch tested?

JsonPathTransformTest.java
- testMultipleDateColumnsWithDifferentFormats() - Verifies multiple date columns with distinct formats are correctly converted using their respective dest_field configurations.
- testMultipleDateColumnsWithNullValues() - Ensures null values in date columns are preserved without causing conversion errors.

Check list

If any new Jar binary package adding in your PR, please add License Notice according
New License Guide
If necessary, please update the documentation to describe the new feature. https://github.com/apache/seatunnel/tree/dev/docs
If necessary, please update incompatible-changes.md to describe the incompatibility caused by this PR.
If you are contributing the connector code, please check that the following files are updated:
1. Update plugin-mapping.properties and add new connector information in it
2. Update the pom file of seatunnel-dist
3. Add ci label in label-scope-conf
4. Add e2e testcase in seatunnel-e2e
5. Update connector plugin_config

…tiple date columns exist

dybyte · 2026-01-23T16:25:16Z

Please enable CI following the instructions.

dybyte

I think we should add null checks in JsonRowConverters#convertToLocalDate and JsonRowConverters#convertToLocalDateTime.
Also, could you add test cases for these two methods in JsonRowDataSerDeSchemaTest?

zhangshenghang · 2026-01-24T12:55:55Z

There is currently a problem with CI, and you need to enable it.

yht0827 · 2026-01-24T14:20:08Z

@dybyte

Thanks for the review!

I’ll add null checks to ‎JsonRowConverters#convertToLocalDate and ‎JsonRowConverters#convertToLocalDateTime, and also add test cases for these methods in ‎JsonRowDataSerDeSchemaTest.

DanielCarter-stack · 2026-01-24T14:27:40Z

Issue 1: Test coverage is not comprehensive enough

Location: seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/JsonPathTransformTest.java:549-662

Related Context:

Test class: JsonPathTransformTest.java
Production class: JsonPathTransform.java:130-165
Dependency class: JsonToRowConverters.java:259-293

Problem Description:

The newly added test cases cover the scenarios of "multiple date columns with different formats" and "containing null values", but are missing the following scenarios:

Regression test for a single date column (to verify the modification does not break original behavior)
Test for multiple date columns with the same format (to verify caching mechanism)
Mixed type field test (to verify it does not affect string, integer, and other types)

Potential Risks:

Risk 1: Unable to ensure compatibility of the modification with single date column scenarios
Risk 2: Unable to verify whether performance optimization for multiple fields with the same format has regressed
Risk 3: Unable to verify the modification does not affect non-date type fields

Impact Scope:

Direct impact: Test coverage of JsonPathTransform
Indirect impact: Safety net for future refactoring
Impact surface: Single Transform (not framework-level)

Severity: MINOR

Improvement Suggestions:

@Test
public void testSingleDateColumn() {
    // Verify that the behavior of a single date column remains consistent with the modifications
    Map<String, Object> configMap = new HashMap<>();
    configMap.put(
        JsonPathTransformConfig.COLUMNS.key(),
        Arrays.asList(
            ImmutableMap.of(
                JsonPathTransformConfig.SRC_FIELD.key(), "data",
                JsonPathTransformConfig.PATH.key(), "$.birth",
                JsonPathTransformConfig.DEST_FIELD.key(), "birth_date",
                JsonPathTransformConfig.DEST_TYPE.key(), "date")));
    // ... test logic
}

@Test
public void testMultipleDateColumnsWithSameFormat() {
    // Verify that multiple date columns with the same format can share cache
    Map<String, Object> configMap = new HashMap<>();
    configMap.put(
        JsonPathTransformConfig.COLUMNS.key(),
        Arrays.asList(
            ImmutableMap.of(..., "birth", "birth_date", "date"),
            ImmutableMap.of(..., "hired", "hire_date", "date")));
    String jsonData = "{\"birth\": \"2024-01-15\", \"hired\": \"2024-02-20\"}";
    // ... verify both fields are correctly parsed
}

@Test
public void testMixedTypeColumns() {
    // Verify mixed configuration of date columns with string and integer columns
    Map<String, Object> configMap = new HashMap<>();
    configMap.put(
        JsonPathTransformConfig.COLUMNS.key(),
        Arrays.asList(
            ImmutableMap.of(..., "date_field", "date"),
            ImmutableMap.of(..., "string_field", "string"),
            ImmutableMap.of(..., "int_field", "int")));
    // ... verify all types are correctly converted
}

Rationale:

These tests can enhance confidence in the correctness of the modification
Single date column test ensures backward compatibility
Same format test verifies cache sharing works properly
Mixed type test verifies isolation of the modification (does not affect other types)

Issue 4: Insufficient encapsulability of fieldFormatterMap

Location: seatunnel-formats/seatunnel-format-json/src/main/java/org/apache/seatunnel/format/json/JsonToRowConverters.java:80

Related Context:

Current definition: public Map<String, DateTimeFormatter> fieldFormatterMap = new HashMap<>();
Usage location: JsonToRowConverters.java:261, 281
Caller: JsonPathTransform.java (indirectly through converter)

Problem Description:

fieldFormatterMap is declared as public, which is an unnecessary encapsulation violation. Although it does need to be accessed externally (e.g., by tests), a better approach is to provide a controlled access method.

Potential Risks:

Risk 1: External code may directly modify the Map, causing cache pollution
Risk 2: Difficult to change internal implementation during future refactoring (e.g., replacing with ConcurrentMap)

Impact Scope:

Direct impact: API design of JsonToRowConverters
Indirect impact: All code using JsonToRowConverters
Impact surface: Format layer (multiple Formats use similar patterns)

Severity: MINOR

Improvement Suggestions:

// Current (line 80)
public Map<String, DateTimeFormatter> fieldFormatterMap = new HashMap<>();

// Suggested change to
private Map<String, DateTimeFormatter> fieldFormatterMap = new HashMap<>();

// If tests need access, add package-level visible access methods
Map<String, DateTimeFormatter> getFieldFormatterMap() {
    return fieldFormatterMap;
}

// Or provide controlled access methods
public DateTimeFormatter getCachedFormatter(String fieldName) {
    return fieldFormatterMap.get(fieldName);
}

Rationale:

Follows encapsulation principle: internal implementation details should not be exposed as public
Prevents accidental modification: external code should not directly manipulate cache
Preserves refactoring freedom: underlying implementation can be replaced in the future without affecting API

Note: This issue is outside the scope of this PR and is an existing problem in the codebase. However, since it was discovered during review, it should be recorded for future improvement.

…tiple date columns exist

…n' into fix/jsonpath-destfield-conversion

…mn conversion

yht0827 · 2026-01-26T02:30:28Z

@dybyte @DanielCarter-stack Thanks for the detailed reviews!

I've added the following test cases to cover the requested scenarios:

JsonPathTransformTest

Test Method	Purpose
`testMultipleDateColumnsWithDifferentFormats`	Multiple date/timestamp columns with different formats
`testMultipleDateColumnsWithNullValues`	Null values don't cause conversion errors
`testSingleDateColumn`	Regression test - single date column still works
`testMultipleDateColumnsWithSameFormat`	Same format columns can share cache
`testMixedTypeColumns`	Mixed types (date + string + int) work correctly
`testInvalidDateFormat`	Invalid date format throws exception
`testEmptyStringDateValue`	Empty string throws exception
`testLeapYearDate`	Leap year (2024-02-29) parses correctly

JsonRowDataSerDeSchemaTest

Test Method	Purpose
`testConvertToLocalDateWithEmptyString`	Empty string handling for LocalDate conversion
`testConvertToLocalDateTimeWithEmptyString`	Empty string handling for LocalDateTime conversion

@DanielCarter-stack I'm also interested in working on issue 4. Will create a follow-up PR after this one is merged.

Please review when you have time.

dybyte · 2026-01-28T18:01:43Z

...eatunnel-format-json/src/main/java/org/apache/seatunnel/format/json/JsonToRowConverters.java

    private LocalDate convertToLocalDate(JsonNode jsonNode, String fieldName) {
        String dateStr = jsonNode.asText();
+
+        if (dateStr == null || dateStr.trim().isEmpty()) {
+            return null;
+        }
+


I think we should check fieldName for null rather than dateStr.
If fieldName is null, we shouldn’t put this entry into fieldFormatterMap.

dybyte · 2026-01-28T18:01:50Z

...eatunnel-format-json/src/main/java/org/apache/seatunnel/format/json/JsonToRowConverters.java

+
+        if (datetimeStr == null || datetimeStr.trim().isEmpty()) {
+            return null;
+        }
+


dybyte · 2026-01-28T18:04:09Z

...l-format-json/src/test/java/org/apache/seatunnel/format/json/JsonRowDataSerDeSchemaTest.java

+    @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));
+    }


Are these tests related to the current PR? I’m not entirely sure about the connection.

dybyte · 2026-01-28T18:13:19Z

seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/JsonPathTransformTest.java

+        Assertions.assertThrows(
+                Exception.class, () -> transform.map(new SeaTunnelRow(new Object[] {jsonData})));


Could we use a more specific exception type here instead of Exception.class?

dybyte · 2026-01-28T18:13:27Z

seatunnel-transforms-v2/src/test/java/org/apache/seatunnel/transform/JsonPathTransformTest.java

+        Assertions.assertThrows(
+                Exception.class, () -> transform.map(new SeaTunnelRow(new Object[] {jsonData})));


[Fix][Transform-V2][JsonPath] Fix date/time conversion error when mul…

1faf117

…tiple date columns exist

github-actions bot added the Transform-v2 label Jan 23, 2026

dybyte reviewed Jan 24, 2026

View reviewed changes

[Fix][Transform-V2][JsonPath] Fix date/time conversion error when mul…

dba5272

…tiple date columns exist

github-actions bot added the format label Jan 25, 2026

yht0827 added 3 commits January 25, 2026 22:02

[Fix][Transform-V2][JsonPath] Fix date/time conversion error when mul…

abf95c9

…tiple date columns exist

Merge remote-tracking branch 'origin/fix/jsonpath-destfield-conversio…

5aac4a1

…n' into fix/jsonpath-destfield-conversion

[Test][Transform-V2][JsonPath] Add edge case tests for date/time colu…

03a69cb

…mn conversion

chore: trigger CI

5f5c5d2

github-actions bot added the Zeta label Jan 28, 2026

Fix: restore accidentally deleted files

c8d3ae3

github-actions bot removed the Zeta label Jan 28, 2026

dybyte reviewed Jan 28, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Fix][Transform-V2][JsonPath] Fix date/time conversion error when mul… #10390

[Fix][Transform-V2][JsonPath] Fix date/time conversion error when mul… #10390

yht0827 commented Jan 23, 2026 •

edited

Loading

Uh oh!

dybyte commented Jan 23, 2026

Uh oh!

dybyte left a comment

Uh oh!

zhangshenghang commented Jan 24, 2026

Uh oh!

yht0827 commented Jan 24, 2026

Uh oh!

DanielCarter-stack commented Jan 24, 2026 •

edited by zhangshenghang

Loading

Uh oh!

yht0827 commented Jan 26, 2026

Uh oh!

dybyte Jan 28, 2026

Uh oh!

dybyte Jan 28, 2026

Uh oh!

dybyte Jan 28, 2026

Uh oh!

dybyte Jan 28, 2026

Uh oh!

dybyte Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

		Assertions.assertThrows(
		Exception.class, () -> transform.map(new SeaTunnelRow(new Object[] {jsonData})));

[Fix][Transform-V2][JsonPath] Fix date/time conversion error when mul… #10390

Are you sure you want to change the base?

[Fix][Transform-V2][JsonPath] Fix date/time conversion error when mul… #10390

Conversation

yht0827 commented Jan 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Purpose of this pull request

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Uh oh!

dybyte commented Jan 23, 2026

Uh oh!

dybyte left a comment

Choose a reason for hiding this comment

Uh oh!

zhangshenghang commented Jan 24, 2026

Uh oh!

yht0827 commented Jan 24, 2026

Uh oh!

DanielCarter-stack commented Jan 24, 2026 • edited by zhangshenghang Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue 1: Test coverage is not comprehensive enough

Issue 4: Insufficient encapsulability of fieldFormatterMap

Uh oh!

yht0827 commented Jan 26, 2026

JsonPathTransformTest

JsonRowDataSerDeSchemaTest

Uh oh!

dybyte Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

dybyte Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

dybyte Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

dybyte Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

dybyte Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

yht0827 commented Jan 23, 2026 •

edited

Loading

DanielCarter-stack commented Jan 24, 2026 •

edited by zhangshenghang

Loading