Hi Mario, Thanks for the KIP! Looks good overall. One quick thought--it wasn't immediately obvious to me why we had to touch on InsertField for this. It looks like the reason is that we use Struct::get [1] to create a clone of the input value [2], instead of Struct::getWithoutDefault [3].
It seems like the pattern of "copy the contents of this Struct into another one for the purpose of mutation" could be fairly common in user code bases in addition to the core Connect SMTs. Do you think there's a way to simplify this with, e.g., a Struct.putAll(Struct destination) or Struct.copy(Schema destinationSchema) method? [1] - https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L78-L91 [2] - https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/transforms/src/main/java/org/apache/kafka/connect/transforms/InsertField.java#L179-L183 [3] - https://github.com/apache/kafka/blob/f74f596bc7d35fcea97edcd83244e5d6aee308d2/connect/api/src/main/java/org/apache/kafka/connect/data/Struct.java#L93-L101 Cheers, Chris On Wed, May 8, 2024 at 3:40 AM Mario Fiore Vitale <mvit...@redhat.com> wrote: > Hi All, > > I have created (through Mickael Maison's account since there was an issue > creating a new one for me) KIP-1040[1] to improve handling of nullable > values in InsertField/ExtractField transformations, this is required after > the introduction of KIP-581[2] that introduced the configuration property > *replace.null.with.default* to *JsonConverter* to choose whether to replace > fields that have a default value and that are null to the default value. > > [1] > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=303794677 > [2] > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value > > Feedback and suggestions are welcome. > > Regards, > Mario. > -- > > Mario Fiore Vitale > > Senior Software Engineer > > Red Hat <https://www.redhat.com/> > <https://www.redhat.com/> >
