Re: [Discuss] KIP-581: Value of optional null field which has default value
t; 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 > > > > > > > > > > > > > > > > >
Re: [Discuss] KIP-581: Value of optional null field which has default value
Thanks Mickael and Cheng! This KIP LGTM! Thanks. Luke On Fri, Mar 17, 2023 at 7:27 PM Mickael Maison wrote: > Thanks Tom and Chris for your feedback! > > I agree one configuration is enough. I've renamed it to > 'replace.null.with.default'. > Since you both seemed happy with the KIP overall, I'll start a vote later > today. > > Thanks, > Mickael > > On Thu, Mar 16, 2023 at 7:08 PM Chris Egerton > wrote: > > > > 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 > 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 > > > 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 > > > > > f
Re: [Discuss] KIP-581: Value of optional null field which has default value
Thanks Tom and Chris for your feedback! I agree one configuration is enough. I've renamed it to 'replace.null.with.default'. Since you both seemed happy with the KIP overall, I'll start a vote later today. Thanks, Mickael On Thu, Mar 16, 2023 at 7:08 PM Chris Egerton wrote: > > 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 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 > > 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 > > > > > 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 > > > > > > > >
Re: [Discuss] KIP-581: Value of optional null field which has default value
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 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 > 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 > > > 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 > > > > >
Re: [Discuss] KIP-581: Value of optional null field which has default value
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 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 > 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 > >
Re: [Discuss] KIP-581: Value of optional null field which has default value
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 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
Re: [Discuss] KIP-581: Value of optional null field which has default value
Hi Cheng Pan, Are you still interested in following up with this KIP? This is a useful feature so it would be nice to get it in Kafka. Thanks, Mickael On Wed, Aug 17, 2022 at 1:39 PM Mickael Maison wrote: > > Hi Cheng Pan, > > Thanks for the updates. I have a few more questions: > > 1) Since this KIP is targeting JsonConverter, I think we should add > this new config to JsonConverterConfig instead of ConverterConfig. > > 2) What about renaming the new field to something like > "use.null.for.optional.fields"? > > Thanks, > Mickael > > On Thu, May 5, 2022 at 3:19 PM Cheng Pan wrote: > > > > Update PR links > > > > [1] https://github.com/apache/kafka/pull/12126 > > > > On 2022/05/05 13:16:59 Cheng Pan wrote: > > > Hi Mickael, > > > > > > Thanks for ping me. > > > > > > I updated the KIP-581 description to narrow the change scope to address > > > this specific issue, and raised a WIP PR[1] to implement it. > > > > > > Please let me know if you have any concerns. > > > > > > [1] https://github.com/apache/kafka/pull/12126 > > > > > > Thanks, > > > Cheng Pan > > > > > > On 2022/05/02 12:57:25 Mickael Maison wrote: > > > > Hi Cheng Pan, > > > > > > > > Thanks for raising this KIP, this would be a useful improvement! > > > > > > > > You never started a VOTE thread for this KIP. Are you interested in > > > > finishing up this work? > > > > If so, based on the discussion, I think you can open a VOTE thread as > > > > described in > > > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#KafkaImprovementProposals-Process > > > > If not, it's not a problem, someone else can volunteer to pick it up. > > > > > > > > Please let us know. > > > > > > > > Thanks, > > > > Mickael > > > > > > > > On Sat, Aug 8, 2020 at 11:05 AM Ruslan Gibaiev > > > > wrote: > > > > > > > > > > Hello guys. > > > > > Proposed PR seems to be fixing the issue in a backward-compatible way. > > > > > Let's please move forward with it. Would be great to see it included > > > > > into next Kafka release > > > > > Thank you > > > > > > > > > > On 2020/07/29 02:49:07, "379377...@qq.com" <379377...@qq.com> wrote: > > > > > > Hi Chris, > > > > > > > > > > > > Thanks for your good suggestion, the KIP document and draft PR has > > > > > > been updated, please review again. > > > > > > > > > > > > And I found due to my misoperation, the mail thread has been > > > > > > broken, no idea how to fix it. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks > > > > > > Cheng Pan > > > > > > > > > > > > From: Christopher Egerton > > > > > > Date: 2020-05-04 10:53 > > > > > > To: dev > > > > > > Subject: Re: [Discuss] KIP-581: Value of optional null field which > > > > > > has default value > > > > > > Hi Cheng, > > > > > > > > > > > > I think refactoring that method should be fine (if maybe a little > > > > > > painful); > > > > > > the method itself is private and all places that it's invoked > > > > > > directly are > > > > > > either package-private or non-static, so it shouldn't affect any of > > > > > > the > > > > > > public methods of the JSON converter to change "convertToConnect" > > > > > > to be > > > > > > non-static. Even if it did, the only parts of the JSON converter > > > > > > that are > > > > > > public API (and therefore officially subject to concerns about > > > > > > compatibility) are the methods it implements that satisfy the > > > > > > "Converter" > > > > > > and "HeaderConverter" interfaces. > > > > > > > > > > > > Would you mind explicitly specifying in the KIP that the new > > > > > > property will > > > > > > be added for the JSON converter only, and that it will affect both > > >
Re: [Discuss] KIP-581: Value of optional null field which has default value
Hi Cheng Pan, Thanks for the updates. I have a few more questions: 1) Since this KIP is targeting JsonConverter, I think we should add this new config to JsonConverterConfig instead of ConverterConfig. 2) What about renaming the new field to something like "use.null.for.optional.fields"? Thanks, Mickael On Thu, May 5, 2022 at 3:19 PM Cheng Pan wrote: > > Update PR links > > [1] https://github.com/apache/kafka/pull/12126 > > On 2022/05/05 13:16:59 Cheng Pan wrote: > > Hi Mickael, > > > > Thanks for ping me. > > > > I updated the KIP-581 description to narrow the change scope to address > > this specific issue, and raised a WIP PR[1] to implement it. > > > > Please let me know if you have any concerns. > > > > [1] https://github.com/apache/kafka/pull/12126 > > > > Thanks, > > Cheng Pan > > > > On 2022/05/02 12:57:25 Mickael Maison wrote: > > > Hi Cheng Pan, > > > > > > Thanks for raising this KIP, this would be a useful improvement! > > > > > > You never started a VOTE thread for this KIP. Are you interested in > > > finishing up this work? > > > If so, based on the discussion, I think you can open a VOTE thread as > > > described in > > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#KafkaImprovementProposals-Process > > > If not, it's not a problem, someone else can volunteer to pick it up. > > > > > > Please let us know. > > > > > > Thanks, > > > Mickael > > > > > > On Sat, Aug 8, 2020 at 11:05 AM Ruslan Gibaiev > > > wrote: > > > > > > > > Hello guys. > > > > Proposed PR seems to be fixing the issue in a backward-compatible way. > > > > Let's please move forward with it. Would be great to see it included > > > > into next Kafka release > > > > Thank you > > > > > > > > On 2020/07/29 02:49:07, "379377...@qq.com" <379377...@qq.com> wrote: > > > > > Hi Chris, > > > > > > > > > > Thanks for your good suggestion, the KIP document and draft PR has > > > > > been updated, please review again. > > > > > > > > > > And I found due to my misoperation, the mail thread has been broken, > > > > > no idea how to fix it. > > > > > > > > > > > > > > > > > > > > > > > > > Thanks > > > > > Cheng Pan > > > > > > > > > > From: Christopher Egerton > > > > > Date: 2020-05-04 10:53 > > > > > To: dev > > > > > Subject: Re: [Discuss] KIP-581: Value of optional null field which > > > > > has default value > > > > > Hi Cheng, > > > > > > > > > > I think refactoring that method should be fine (if maybe a little > > > > > painful); > > > > > the method itself is private and all places that it's invoked > > > > > directly are > > > > > either package-private or non-static, so it shouldn't affect any of > > > > > the > > > > > public methods of the JSON converter to change "convertToConnect" to > > > > > be > > > > > non-static. Even if it did, the only parts of the JSON converter that > > > > > are > > > > > public API (and therefore officially subject to concerns about > > > > > compatibility) are the methods it implements that satisfy the > > > > > "Converter" > > > > > and "HeaderConverter" interfaces. > > > > > > > > > > Would you mind explicitly specifying in the KIP that the new property > > > > > will > > > > > be added for the JSON converter only, and that it will affect both > > > > > serialization and deserialization? > > > > > > > > > > Cheers, > > > > > > > > > > Chris > > > > > > > > > > On Tue, Apr 28, 2020 at 10:52 AM 379377944 <379377...@qq.com> wrote: > > > > > > > > > > > Hi Chris, > > > > > > > > > > > > > > > > > > Thanks for your reminder, the original implement is deprecated, I > > > > > > just > > > > > > update the JIRA with the new > > > > > > PR link: https://github.com/apache/kafka/pull/8575 > > > > > &
Re: [Discuss] KIP-581: Value of optional null field which has default value
Update PR links [1] https://github.com/apache/kafka/pull/12126 On 2022/05/05 13:16:59 Cheng Pan wrote: > Hi Mickael, > > Thanks for ping me. > > I updated the KIP-581 description to narrow the change scope to address this > specific issue, and raised a WIP PR[1] to implement it. > > Please let me know if you have any concerns. > > [1] https://github.com/apache/kafka/pull/12126 > > Thanks, > Cheng Pan > > On 2022/05/02 12:57:25 Mickael Maison wrote: > > Hi Cheng Pan, > > > > Thanks for raising this KIP, this would be a useful improvement! > > > > You never started a VOTE thread for this KIP. Are you interested in > > finishing up this work? > > If so, based on the discussion, I think you can open a VOTE thread as > > described in > > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#KafkaImprovementProposals-Process > > If not, it's not a problem, someone else can volunteer to pick it up. > > > > Please let us know. > > > > Thanks, > > Mickael > > > > On Sat, Aug 8, 2020 at 11:05 AM Ruslan Gibaiev wrote: > > > > > > Hello guys. > > > Proposed PR seems to be fixing the issue in a backward-compatible way. > > > Let's please move forward with it. Would be great to see it included into > > > next Kafka release > > > Thank you > > > > > > On 2020/07/29 02:49:07, "379377...@qq.com" <379377...@qq.com> wrote: > > > > Hi Chris, > > > > > > > > Thanks for your good suggestion, the KIP document and draft PR has been > > > > updated, please review again. > > > > > > > > And I found due to my misoperation, the mail thread has been broken, no > > > > idea how to fix it. > > > > > > > > > > > > > > > > > > > > Thanks > > > > Cheng Pan > > > > > > > > From: Christopher Egerton > > > > Date: 2020-05-04 10:53 > > > > To: dev > > > > Subject: Re: [Discuss] KIP-581: Value of optional null field which has > > > > default value > > > > Hi Cheng, > > > > > > > > I think refactoring that method should be fine (if maybe a little > > > > painful); > > > > the method itself is private and all places that it's invoked directly > > > > are > > > > either package-private or non-static, so it shouldn't affect any of the > > > > public methods of the JSON converter to change "convertToConnect" to be > > > > non-static. Even if it did, the only parts of the JSON converter that > > > > are > > > > public API (and therefore officially subject to concerns about > > > > compatibility) are the methods it implements that satisfy the > > > > "Converter" > > > > and "HeaderConverter" interfaces. > > > > > > > > Would you mind explicitly specifying in the KIP that the new property > > > > will > > > > be added for the JSON converter only, and that it will affect both > > > > serialization and deserialization? > > > > > > > > Cheers, > > > > > > > > Chris > > > > > > > > On Tue, Apr 28, 2020 at 10:52 AM 379377944 <379377...@qq.com> wrote: > > > > > > > > > Hi Chris, > > > > > > > > > > > > > > > Thanks for your reminder, the original implement is deprecated, I just > > > > > update the JIRA with the new > > > > > PR link: https://github.com/apache/kafka/pull/8575 > > > > > > > > > > > > > > > As question 2), I agree with you that we should consider both > > > > > serialization and deserialization, and as you said, I only implement > > > > > the > > > > > serialization now. This is because the original serde implement is > > > > > not > > > > > symmetrical, the convertToConnect is a static method and can’t access > > > > > the > > > > > field in JsonConverter > > > > > instance, maybe I should do some refactoring to implement the > > > > > deserialization. > > > > > > > > > > > > > > > Thanks, > > > > > Cheng Pan > > > > > Original Message > > > > > Sender: Christopher Egerton > > > > > Recipient: dev > >
Re: [Discuss] KIP-581: Value of optional null field which has default value
Hi Mickael, Thanks for ping me. I updated the KIP-581 description to narrow the change scope to address this specific issue, and raised a WIP PR[1] to implement it. Please let me know if you have any concerns. [1] https://github.com/apache/kafka/pull/8575 Thanks, Cheng Pan On 2022/05/02 12:57:25 Mickael Maison wrote: > Hi Cheng Pan, > > Thanks for raising this KIP, this would be a useful improvement! > > You never started a VOTE thread for this KIP. Are you interested in > finishing up this work? > If so, based on the discussion, I think you can open a VOTE thread as > described in > https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#KafkaImprovementProposals-Process > If not, it's not a problem, someone else can volunteer to pick it up. > > Please let us know. > > Thanks, > Mickael > > On Sat, Aug 8, 2020 at 11:05 AM Ruslan Gibaiev wrote: > > > > Hello guys. > > Proposed PR seems to be fixing the issue in a backward-compatible way. > > Let's please move forward with it. Would be great to see it included into > > next Kafka release > > Thank you > > > > On 2020/07/29 02:49:07, "379377...@qq.com" <379377...@qq.com> wrote: > > > Hi Chris, > > > > > > Thanks for your good suggestion, the KIP document and draft PR has been > > > updated, please review again. > > > > > > And I found due to my misoperation, the mail thread has been broken, no > > > idea how to fix it. > > > > > > > > > > > > > > > Thanks > > > Cheng Pan > > > > > > From: Christopher Egerton > > > Date: 2020-05-04 10:53 > > > To: dev > > > Subject: Re: [Discuss] KIP-581: Value of optional null field which has > > > default value > > > Hi Cheng, > > > > > > I think refactoring that method should be fine (if maybe a little > > > painful); > > > the method itself is private and all places that it's invoked directly are > > > either package-private or non-static, so it shouldn't affect any of the > > > public methods of the JSON converter to change "convertToConnect" to be > > > non-static. Even if it did, the only parts of the JSON converter that are > > > public API (and therefore officially subject to concerns about > > > compatibility) are the methods it implements that satisfy the "Converter" > > > and "HeaderConverter" interfaces. > > > > > > Would you mind explicitly specifying in the KIP that the new property will > > > be added for the JSON converter only, and that it will affect both > > > serialization and deserialization? > > > > > > Cheers, > > > > > > Chris > > > > > > On Tue, Apr 28, 2020 at 10:52 AM 379377944 <379377...@qq.com> wrote: > > > > > > > Hi Chris, > > > > > > > > > > > > Thanks for your reminder, the original implement is deprecated, I just > > > > update the JIRA with the new > > > > PR link: https://github.com/apache/kafka/pull/8575 > > > > > > > > > > > > As question 2), I agree with you that we should consider both > > > > serialization and deserialization, and as you said, I only implement the > > > > serialization now. This is because the original serde implement is not > > > > symmetrical, the convertToConnect is a static method and can’t access > > > > the > > > > field in JsonConverter > > > > instance, maybe I should do some refactoring to implement the > > > > deserialization. > > > > > > > > > > > > Thanks, > > > > Cheng Pan > > > > Original Message > > > > Sender: Christopher Egerton > > > > Recipient: dev > > > > Date: Wednesday, Apr 15, 2020 02:28 > > > > Subject: Re: [Discuss] KIP-581: Value of optional null field which has > > > > default value > > > > > > > > > > > > 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
Re: Re: [Discuss] KIP-581: Value of optional null field which has default value
Hi Cheng Pan, Thanks for raising this KIP, this would be a useful improvement! You never started a VOTE thread for this KIP. Are you interested in finishing up this work? If so, based on the discussion, I think you can open a VOTE thread as described in https://cwiki.apache.org/confluence/display/KAFKA/Kafka+Improvement+Proposals#KafkaImprovementProposals-Process If not, it's not a problem, someone else can volunteer to pick it up. Please let us know. Thanks, Mickael On Sat, Aug 8, 2020 at 11:05 AM Ruslan Gibaiev wrote: > > Hello guys. > Proposed PR seems to be fixing the issue in a backward-compatible way. > Let's please move forward with it. Would be great to see it included into > next Kafka release > Thank you > > On 2020/07/29 02:49:07, "379377...@qq.com" <379377...@qq.com> wrote: > > Hi Chris, > > > > Thanks for your good suggestion, the KIP document and draft PR has been > > updated, please review again. > > > > And I found due to my misoperation, the mail thread has been broken, no > > idea how to fix it. > > > > > > > > > > Thanks > > Cheng Pan > > > > From: Christopher Egerton > > Date: 2020-05-04 10:53 > > To: dev > > Subject: Re: [Discuss] KIP-581: Value of optional null field which has > > default value > > Hi Cheng, > > > > I think refactoring that method should be fine (if maybe a little painful); > > the method itself is private and all places that it's invoked directly are > > either package-private or non-static, so it shouldn't affect any of the > > public methods of the JSON converter to change "convertToConnect" to be > > non-static. Even if it did, the only parts of the JSON converter that are > > public API (and therefore officially subject to concerns about > > compatibility) are the methods it implements that satisfy the "Converter" > > and "HeaderConverter" interfaces. > > > > Would you mind explicitly specifying in the KIP that the new property will > > be added for the JSON converter only, and that it will affect both > > serialization and deserialization? > > > > Cheers, > > > > Chris > > > > On Tue, Apr 28, 2020 at 10:52 AM 379377944 <379377...@qq.com> wrote: > > > > > Hi Chris, > > > > > > > > > Thanks for your reminder, the original implement is deprecated, I just > > > update the JIRA with the new > > > PR link: https://github.com/apache/kafka/pull/8575 > > > > > > > > > As question 2), I agree with you that we should consider both > > > serialization and deserialization, and as you said, I only implement the > > > serialization now. This is because the original serde implement is not > > > symmetrical, the convertToConnect is a static method and can’t access the > > > field in JsonConverter > > > instance, maybe I should do some refactoring to implement the > > > deserialization. > > > > > > > > > Thanks, > > > Cheng Pan > > > Original Message > > > Sender: Christopher Egerton > > > Recipient: dev > > > Date: Wednesday, Apr 15, 2020 02:28 > > > Subject: Re: [Discuss] KIP-581: Value of optional null field which has > > > default value > > > > > > > > > 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 > >
Re: Re: [Discuss] KIP-581: Value of optional null field which has default value
Hello guys. Proposed PR seems to be fixing the issue in a backward-compatible way. Let's please move forward with it. Would be great to see it included into next Kafka release Thank you On 2020/07/29 02:49:07, "379377...@qq.com" <379377...@qq.com> wrote: > Hi Chris, > > Thanks for your good suggestion, the KIP document and draft PR has been > updated, please review again. > > And I found due to my misoperation, the mail thread has been broken, no idea > how to fix it. > > > > > Thanks > Cheng Pan > > From: Christopher Egerton > Date: 2020-05-04 10:53 > To: dev > Subject: Re: [Discuss] KIP-581: Value of optional null field which has > default value > Hi Cheng, > > I think refactoring that method should be fine (if maybe a little painful); > the method itself is private and all places that it's invoked directly are > either package-private or non-static, so it shouldn't affect any of the > public methods of the JSON converter to change "convertToConnect" to be > non-static. Even if it did, the only parts of the JSON converter that are > public API (and therefore officially subject to concerns about > compatibility) are the methods it implements that satisfy the "Converter" > and "HeaderConverter" interfaces. > > Would you mind explicitly specifying in the KIP that the new property will > be added for the JSON converter only, and that it will affect both > serialization and deserialization? > > Cheers, > > Chris > > On Tue, Apr 28, 2020 at 10:52 AM 379377944 <379377...@qq.com> wrote: > > > Hi Chris, > > > > > > Thanks for your reminder, the original implement is deprecated, I just > > update the JIRA with the new > > PR link: https://github.com/apache/kafka/pull/8575 > > > > > > As question 2), I agree with you that we should consider both > > serialization and deserialization, and as you said, I only implement the > > serialization now. This is because the original serde implement is not > > symmetrical, the convertToConnect is a static method and can’t access the > > field in JsonConverter > > instance, maybe I should do some refactoring to implement the > > deserialization. > > > > > > Thanks, > > Cheng Pan > > Original Message > > Sender: Christopher Egerton > > Recipient: dev > > Date: Wednesday, Apr 15, 2020 02:28 > > Subject: Re: [Discuss] KIP-581: Value of optional null field which has > > default value > > > > > > 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 >
Re: Re: [Discuss] KIP-581: Value of optional null field which has default value
Hi Chris, Thanks for your good suggestion, the KIP document and draft PR has been updated, please review again. And I found due to my misoperation, the mail thread has been broken, no idea how to fix it. Thanks Cheng Pan From: Christopher Egerton Date: 2020-05-04 10:53 To: dev Subject: Re: [Discuss] KIP-581: Value of optional null field which has default value Hi Cheng, I think refactoring that method should be fine (if maybe a little painful); the method itself is private and all places that it's invoked directly are either package-private or non-static, so it shouldn't affect any of the public methods of the JSON converter to change "convertToConnect" to be non-static. Even if it did, the only parts of the JSON converter that are public API (and therefore officially subject to concerns about compatibility) are the methods it implements that satisfy the "Converter" and "HeaderConverter" interfaces. Would you mind explicitly specifying in the KIP that the new property will be added for the JSON converter only, and that it will affect both serialization and deserialization? Cheers, Chris On Tue, Apr 28, 2020 at 10:52 AM 379377944 <379377...@qq.com> wrote: > Hi Chris, > > > Thanks for your reminder, the original implement is deprecated, I just > update the JIRA with the new > PR link: https://github.com/apache/kafka/pull/8575 > > > As question 2), I agree with you that we should consider both > serialization and deserialization, and as you said, I only implement the > serialization now. This is because the original serde implement is not > symmetrical, the convertToConnect is a static method and can’t access the > field in JsonConverter > instance, maybe I should do some refactoring to implement the > deserialization. > > > Thanks, > Cheng Pan > Original Message > Sender: Christopher Egerton > Recipient: dev > Date: Wednesday, Apr 15, 2020 02:28 > Subject: Re: [Discuss] KIP-581: Value of optional null field which has > default value > > > 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
Re: [Discuss] KIP-581: Value of optional null field which has default value
Hi Cheng, I think refactoring that method should be fine (if maybe a little painful); the method itself is private and all places that it's invoked directly are either package-private or non-static, so it shouldn't affect any of the public methods of the JSON converter to change "convertToConnect" to be non-static. Even if it did, the only parts of the JSON converter that are public API (and therefore officially subject to concerns about compatibility) are the methods it implements that satisfy the "Converter" and "HeaderConverter" interfaces. Would you mind explicitly specifying in the KIP that the new property will be added for the JSON converter only, and that it will affect both serialization and deserialization? Cheers, Chris On Tue, Apr 28, 2020 at 10:52 AM 379377944 <379377...@qq.com> wrote: > Hi Chris, > > > Thanks for your reminder, the original implement is deprecated, I just > update the JIRA with the new > PR link: https://github.com/apache/kafka/pull/8575 > > > As question 2), I agree with you that we should consider both > serialization and deserialization, and as you said, I only implement the > serialization now. This is because the original serde implement is not > symmetrical, the convertToConnect is a static method and can’t access the > field in JsonConverter > instance, maybe I should do some refactoring to implement the > deserialization. > > > Thanks, > Cheng Pan > Original Message > Sender: Christopher Egerton > Recipient: dev > Date: Wednesday, Apr 15, 2020 02:28 > Subject: Re: [Discuss] KIP-581: Value of optional null field which has > default value > > > 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
Re: [Discuss] KIP-581: Value of optional null field which has default value
Hi Chris, Thanks for your reminder, the original implement is deprecated, I just update the JIRA with the new PR link: https://github.com/apache/kafka/pull/8575 As question 2), I agree with you that we should consider both serialization and deserialization, and as you said, I only implement the serialization now. This is because the original serde implement is not symmetrical, the convertToConnect is a static method and can’t access the field in JsonConverter instance, maybe I should do some refactoring to implement the deserialization. Thanks, Cheng Pan Original Message Sender: Christopher Egerton Recipient: dev Date: Wednesday, Apr 15, 2020 02:28 Subject: Re: [Discuss] KIP-581: Value of optional null field which has default value 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
Re: [Discuss] KIP-581: Value of optional null field which has default value
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
[Discuss] KIP-581: Value of optional null field which has default value
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
