zhangshenghang commented on PR #10335:
URL: https://github.com/apache/seatunnel/pull/10335#issuecomment-3755044596

   <!-- code-pr-reviewer -->
   Thanks for the contribution! However, there are critical issues that must be 
addressed before merge.
   
   **[P0] Header fields with null values are permanently lost (COR-001)**
   - `DefaultSeaTunnelRowSerializer.java:223` skips null values when writing 
headers
   - `DefaultSeaTunnelRowSerializer.java:350-355` unconditionally excludes all 
headerFields from value regardless of null values
   - Impact: Fields with null values disappear from both headers and value with 
no warning. Consider writing "null" string (consistent with 
partition_key_fields behavior), or retaining null fields in value.
   
   **[MAJOR] No overlap validation between partition_key_fields and 
kafka_headers_fields (GEN-001)**
   - `KafkaSinkWriter.java:252-290` validates both configs independently 
without checking for intersection
   - Impact: Same field can appear in both key and headers, then be excluded 
from value, causing data loss and confusion. Add validation to reject 
overlapping fields (similar to existing L197-201 pattern).
   
   **[MINOR] NATIVE format silently ignores kafka_headers_fields (GEN-002)**
   - `KafkaSinkWriter.java:187-190` calls a create method that doesn't support 
headerFields parameter
   - Impact: Configuration takes effect with no error but headers are not 
written. Either add validation to reject the config for NATIVE format, or 
document this limitation clearly.
   
   Please fix at least the P0 and MAJOR issues before merging.


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