Request Permission to Create KIP
Hi all, I??m writing to request permission to create KIP. There is some discuss at https://issues.apache.org/jira/browse/KAFKA-8713. I'd like to come up a KIP to push on it. 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
Re: [DISCUSS] KIP-581: add tag "partition" to BrokerTopicMetrics so as to observe the partition metrics on the same broker
Hi Chia-Ping, I'm afraid the KIP number is conflict. I created (KIP-581: Value of optional null field which has default value) yesterday early, and insert it into table [KIPs under discussion]. https://cwiki.apache.org/confluence/display/KAFKA/KIP-581%3A+Value+of+optional+null+field+which+has+default+value Would you please choose another KIP number, maybe 583, I also see KIP-582 discussion thread appears in the mail list, and insert into table [KIPs under discussion]. Best Regards, Cheng Pan -- Original -- From: "Chia-Ping Tsai";; Send time: Thursday, Mar 19, 2020 1:48 AM To: "dev"; Subject: [DISCUSS] KIP-581: add tag "partition" to BrokerTopicMetrics so as to observe the partition metrics on the same broker hi this ticket is about to records partition metrics rather than topic metrics. It helps us to observe more precis metrics for specify partition. The downside is that broker needs more space to keep metrics and the origin metrics interface (tags) is broken since this ticket adds new tag "partition=xxx" to it. The alternative is an new config flag used to enable new metrics (of course, it is false by default). Also, we ought to provide enough document to explain benefit and cost of new metrics. KIP-581 (https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=148642648) JIRA (https://issues.apache.org/jira/browse/KAFKA-9730) Please take a look :) Best Regards, Chia-Ping
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 > >
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 > > > > > Rec
[jira] [Created] (KAFKA-8713) Connect JsonConverter swallow null value on optional filed which has default value
Cheng Pan created KAFKA-8713:
Summary: Connect JsonConverter swallow null value on optional
filed which has default value
Key: KAFKA-8713
URL: https://issues.apache.org/jira/browse/KAFKA-8713
Project: Kafka
Issue Type: Bug
Components: KafkaConnect
Affects Versions: 2.2.1, 2.3.0, 2.2.0, 2.1.0, 2.0.1, 2.0.0
Reporter: Cheng Pan
Class JsonConverter line: 582
{code:java}
private static JsonNode convertToJson(Schema schema, Object logicalValue) {
if (logicalValue == null) {
if (schema == null) // Any schema is valid and we don't have a
default, so treat this as an optional schema
return null;
if (schema.defaultValue() != null)
return convertToJson(schema, schema.defaultValue());
if (schema.isOptional())
return JsonNodeFactory.instance.nullNode();
throw new DataException("Conversion error: null value for field
that is required and has no default value");
}
}
{code}
h1.The Excepted:
Value `null` is valid for an optional filed, even through the filed has a
default value.
Only when field is required, the converter return default value fallback when
value is `null`.
h1.The Current:
Always return default value if `null` was given.
h1. Addition:
I'm not sure if the current behavior is the exactly expected, but at least on
MySQL, a table define as
{code:sql}
create table t1 {
name varchar(40) not null,
create_time datetime default '1999-01-01 11:11:11' null,
update_time datetime default '1999-01-01 11:11:11' null,
}
{code}
Just insert a record:
{code:sql}
INSERT INTO `t1` (`name`, `update_time`) VALUES ('kafka', null);
{code}
The result is:
{code:json}
{
"name": "kafka",
"create_time": "1999-01-01 11:11:11",
"update_time": null
}
{code}
But when I use debezium pull binlog and send the record to Kafka with
JsonConverter, the result changed to:
{code:json}
{
"name": "kafka",
"create_time": "1999-01-01 11:11:11",
"update_time": "1999-01-01 11:11:11"
}
{code}
--
This message was sent by Atlassian JIRA
(v7.6.14#76016)
