yashmayya commented on code in PR #14024: URL: https://github.com/apache/kafka/pull/14024#discussion_r1266957728
########## connect/api/src/main/java/org/apache/kafka/connect/sink/SinkRecord.java: ########## @@ -65,7 +180,8 @@ public SinkRecord newRecord(String topic, Integer kafkaPartition, Schema keySche @Override public SinkRecord newRecord(String topic, Integer kafkaPartition, Schema keySchema, Object key, Schema valueSchema, Object value, Long timestamp, Iterable<Header> headers) { - return new SinkRecord(topic, kafkaPartition, keySchema, key, valueSchema, value, kafkaOffset(), timestamp, timestampType, headers); + return new SinkRecord(topic, kafkaPartition, keySchema, key, valueSchema, value, kafkaOffset(), timestamp, timestampType, headers, + originalTopic(), originalKafkaPartition(), originalKafkaOffset()); Review Comment: Haha wow I completely glossed over that, nice catch. > It'd be great if we could fix that bug (and possibly add testing coverage for it) I think it might be worth filing a separate bug ticket and PR to fix that so that we could also backport it as well? We did discuss whether or not the method is even useful [here](https://github.com/apache/kafka/pull/14024#discussion_r1265694650), but given that we technically do support such use cases (and can't prove that it isn't used), it'd probably be good to backport a fix for such a bad bug if possible. WDYT? > And IMO this is a decent reason to abandon using methods to differentiate between fields and parameters Yeah, I'm definitely convinced since I missed it as well 😆 ########## connect/api/src/main/java/org/apache/kafka/connect/sink/SinkRecord.java: ########## @@ -65,7 +180,8 @@ public SinkRecord newRecord(String topic, Integer kafkaPartition, Schema keySche @Override public SinkRecord newRecord(String topic, Integer kafkaPartition, Schema keySchema, Object key, Schema valueSchema, Object value, Long timestamp, Iterable<Header> headers) { - return new SinkRecord(topic, kafkaPartition, keySchema, key, valueSchema, value, kafkaOffset(), timestamp, timestampType, headers); + return new SinkRecord(topic, kafkaPartition, keySchema, key, valueSchema, value, kafkaOffset(), timestamp, timestampType, headers, + originalTopic(), originalKafkaPartition(), originalKafkaOffset()); Review Comment: Haha wow I completely glossed over that, nice catch. > It'd be great if we could fix that bug (and possibly add testing coverage for it) I think it might be worth filing a separate bug ticket and PR to fix that so that we could also backport it as well? We did discuss whether or not the method is even useful [here](https://github.com/apache/kafka/pull/14024#discussion_r1265694650), but given that we technically do support such use cases (and can't prove that it isn't used), it'd probably be good to backport a fix for such a bad bug if possible. WDYT? > And IMO this is a decent reason to abandon using methods to differentiate between fields and parameters Yeah, I'm definitely convinced since I missed it as well 😆 -- 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: jira-unsubscr...@kafka.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org