Hi Mickael, Tom got this part perfect so I'm just going to copy+paste: Thanks to Cheng for the KIP and to you for picking it up.
I'm wondering why we need separate properties for serialization vs. deserialization? If the converter is being used by the Kafka Connect runtime, a given instance of it will only ever be responsible for one or the other (in other words, sink connectors' converters will only ever be used for deserialization and source connectors' converters will only ever be used for serialization). It seems like a single "use.optional.null" (or "map.null.to.default") property would be simpler and easier to use, but I might be missing something about why we'd want to add this kind of granularity. I'm slightly in favor of the alternative name that Tom has proposed. I think highlighting that this property deals with how to handle default values is important, possibly more important than the fact that it deals with null field values. I'm a little hesitant to use "map" since that may be harder to remember and at first glance it might seem like it deals with the map schema type. Maybe "replace.null.with.default.value"? (For the record, I don't consider any of this worthy of blocking the KIP, so don't feel the need to appease me on this front before starting a vote.) Cheers, Chris On Thu, Mar 16, 2023 at 11:38 AM Tom Bentley <tbent...@redhat.com> wrote: > Hi Mickael, > > Thanks to Cheng for the KIP and to you for picking it up. > > My only comment (feel free to ignore) is about the names of the configs. > Personally I don't think I'd correctly guess what > "serialize.use.optional.null" meant. Something like > "serialize.map.null.to.default" is much clearer to me, for the cost of one > extra token. > > Otherwise LGTM. > > Thanks, > > Tom > > On Wed, 8 Mar 2023 at 15:55, Mickael Maison <mickael.mai...@gmail.com> > wrote: > > > Hi, > > > > This KIP has been staled for a long time. Since it would be a useful > > feature, I pinged Cheng about a month ago asking if he was planning to > > work on it again. I've not received a reply, so I've allowed myself to > > update the KIP (hopefully preserving the initial requirements) and > > would like to restart a discussion. > > > > The DISCUSS thread was split in two, you can find the other part in > > https://lists.apache.org/thread/dc56k17zptzvbyc7gtscovzgzwf6yn9p > > > > Let me know if you have any feedback. > > > > Thanks, > > Mickael > > > > On Tue, Apr 14, 2020 at 8:28 PM Christopher Egerton <chr...@confluent.io > > > > wrote: > > > > > > Hi Cheng, > > > > > > Thanks for the KIP! I really appreciate the care that was taken to > ensure > > > backwards compatibility for existing users, and the minimal changes to > > > public interface that are suggested to address this. > > > > > > I have two quick requests for clarification: > > > > > > 1) Where is the proposed "accept.optional.null" property going to > apply? > > > It's hinted that it'll take effect on the JSON converter but not > actually > > > called out anywhere. > > > > > > 2) Assuming this takes effect on the JSON converter, is the intent to > > alter > > > the semantics for both serialization and deserialization? The code > > snippet > > > from the JSON converter that's included in the KIP comes from the > > > "convertToJson" method, which is used for serialization. However, based > > on > > > > > > https://github.com/apache/kafka/blob/ea47a885b1fe47dfb87c1dc86db1b0e7eb8a273c/connect/json/src/main/java/org/apache/kafka/connect/json/JsonConverter.java#L712-L713 > > > it > > > looks like the converter also inserts the default value for > > > optional-but-null data during deserialization. > > > > > > Thanks again for the KIP! > > > > > > Cheers, > > > > > > Chris > > > > > > On Wed, Mar 18, 2020 at 12:00 AM Cheng Pan <379377...@qq.com> wrote: > > > > > > > Hi all, > > > > > > > > I'd like to use this thread to discuss KIP-581: Value of optional > null > > > > field which has default value, please see detail at: > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value > > > > > > > > > > > > There are some previous discussion at: > > > > https://github.com/apache/kafka/pull/7112 > > > > > > > > > > > > I'm a beginner for apache project, please let me know if I did any > > thing > > > > wrong. > > > > > > > > > > > > Best regards, > > > > Cheng Pan > > > > >
