[jira] [Commented] (KAFKA-9763) Recent changes to Connect's InsertField will fail to inject field on key of tombstone record

["ASF GitHub Bot (Jira)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/KAFKA-9763?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17067001#comment-17067001 ] 

ASF GitHub Bot commented on KAFKA-9763:
---------------------------------------

rhauch commented on pull request #8351: KAFKA-9763: Correct InsertField to not skip keys of tombstone records
URL: https://github.com/apache/kafka/pull/8351
 
 
   The fix for [KAFKA-8523](https://issues.apache.org/jira/browse/KAFKA-8523) attempted to skip (immediately returns) tombstone records when InsertField$Value is used. However, it also inadvertently also skips tombstone records when InsertField$Key is used, despite the possibility that the tombstone record’s key is non null.
   
   This commit corrects the behavior so that the InsertField$Value continues to skip tombstone records, but InsertField$Key only skips when the record’s *key* is null.
   
   Added two unit tests, and verified that these tests fail without the proposed fix and pass with the proposed fix. The other unit tests were added with [KAFKA-8523](https://issues.apache.org/jira/browse/KAFKA-8523) continue to pass.
   
   This should be backported all the way back to the 1.0 branch, since that's how far back [KAFKA-8523](https://issues.apache.org/jira/browse/KAFKA-8523) was backported.
   
   ### Committer Checklist (excluded from commit message)
   - [ ] Verify design and implementation 
   - [ ] Verify test coverage and CI build status
   - [ ] Verify documentation (including upgrade notes)
   
 
----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


> Recent changes to Connect's InsertField will fail to inject field on key of tombstone record
> --------------------------------------------------------------------------------------------
>
>                 Key: KAFKA-9763
>                 URL: https://issues.apache.org/jira/browse/KAFKA-9763
>             Project: Kafka
>          Issue Type: Bug
>          Components: KafkaConnect
>    Affects Versions: 1.0.3, 1.1.2, 2.0.2, 2.1.2, 2.2.2, 2.4.0, 2.3.1, 2.5.0
>            Reporter: Randall Hauch
>            Assignee: Randall Hauch
>            Priority: Major
>             Fix For: 1.0.3, 1.1.2, 2.0.2, 2.1.2, 2.2.3, 2.3.2, 2.4.2, 2.5.1
>
>
> This is a regression due to the changes for KAFKA-8523.
> KAFKA-8523 was backported to multiple versions, and was released into 2.2.2, 2.3.1, and 2.4.0, and will soon be released in 2.5.0.
> Unfortunately, that fix always makes the `InsertField` SMT skip all tombstone records, even when using the `InsertField$Key`.
> Rather than:
> {code:java}
>     private boolean isTombstoneRecord(R record) {
>         return record.value() == null;
>     }
> {code}
> the correct behavior would be:
> {code:java}
>      private boolean isTombstoneRecord(R record) {
>          return operatingValue(record) == null;
>      }
> {code}
> The method no longer detects just tombstone methods, so the code should be refactored to return the record if the operatingValue for the record (which for `InsertField$Key` is the record key and for `InsertField$Value` is the record value) is null.
>  



--
This message was sent by Atlassian Jira
(v8.3.4#803005)