You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Cheng Pan <37...@qq.com> on 2020/03/18 07:00:23 UTC
[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
Re: [Discuss] KIP-581: Value of optional null field which has default value
Posted by Chris Egerton <ch...@aiven.io.INVALID>.
LGTM
On Fri, Mar 17, 2023, 08:26 Luke Chen <sh...@gmail.com> wrote:
> Thanks Mickael and Cheng!
> This KIP LGTM!
>
> Thanks.
> Luke
>
> On Fri, Mar 17, 2023 at 7:27 PM Mickael Maison <mi...@gmail.com>
> 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 <ch...@aiven.io.invalid>
> > 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 <tb...@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.maison@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 <
> > chrise@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 <37...@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
Posted by Luke Chen <sh...@gmail.com>.
Thanks Mickael and Cheng!
This KIP LGTM!
Thanks.
Luke
On Fri, Mar 17, 2023 at 7:27 PM Mickael Maison <mi...@gmail.com>
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 <ch...@aiven.io.invalid>
> 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 <tb...@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 <mi...@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 <
> chrise@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 <37...@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
Posted by Mickael Maison <mi...@gmail.com>.
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 <ch...@aiven.io.invalid> 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 <tb...@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 <mi...@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 <chrise@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 <37...@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
Posted by Chris Egerton <ch...@aiven.io.INVALID>.
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 <tb...@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 <mi...@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 <chrise@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 <37...@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
Posted by Tom Bentley <tb...@redhat.com>.
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 <mi...@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 <ch...@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 <37...@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
Posted by Mickael Maison <mi...@gmail.com>.
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 <ch...@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 <37...@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
Posted by Christopher Egerton <ch...@confluent.io>.
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 <37...@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