featzhang commented on PR #27897: URL: https://github.com/apache/flink/pull/27897#issuecomment-4257059004
## Review Comments Addressed ### ✅ Fixed comments (from @RocMarshal) | # | File | Line | Fix | Status | |---|------|------|-----|--------| | 1 | RawFormatLineDelimiterTest.java | 61 | Use `StandardCharsets.UTF_8` instead of string literal `"UTF-8"` | ✅ | | 2 | RawFormatLineDelimiterTest.java | 63 | Simplify assertion using `hasToString()` | ✅ | | 3 | RawFormatDeserializationSchema.java | 68 | Add note that `lineDelimiter` and `lineDelimiterPattern` are either both null or both non-null | ✅ | | 4 | RawFormatDeserializationSchema.java | 131 | Design explanation for null message handling (see below) | 💬 | ### 📁 Modified files 1. **RawFormatLineDelimiterTest.java**: Added `StandardCharsets` import, replaced 7 string literals, simplified 1 assertion using `hasToString()` 2. **RawFormatDeserializationSchema.java**: Added nullability relationship comment for `lineDelimiter` and `lineDelimiterPattern` fields ### 💬 Design note for line 131 (null message with delimiter) The current behavior (returning early on `null` message when a delimiter is set) is intentional and differs from the single-record path for the following reasons: - When `lineDelimiter` is set, the semantics shift from "one message → one row" to "one message → N rows via splitting". A `null` message in this context means there is no content to split, so emitting zero rows is the correct and expected result. - Emitting a row with a `null` field for a `null` message would be inconsistent with the splitting contract: if the delimiter is `\n` and the message is `null`, what does it mean to split `null` by `\n`? Producing a null row conflates "no data" with "one empty/null segment". - The existing test `testDeserializeNullMessageWithDelimiter_noRows` explicitly documents and verifies this design decision. @RocMarshal PTAL, thanks! -- 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]
