You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "fanyang (via GitHub)" <gi...@apache.org> on 2024/05/23 05:29:38 UTC
[PR] Improve logical type compatibility in SchemaProjector [kafka]
fanyang opened a new pull request, #16035:
URL: https://github.com/apache/kafka/pull/16035
This PR improved logical type compatibility in SchemaProjector.
Decimal type with higher scale is compatible with one with same or lower scale.
Timestamp type is compatible with Date and Time type.
Date and Time types are incompatible with each other.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16257: Improve logical type compatibility in SchemaProjector [kafka]
Posted by "fanyang (via GitHub)" <gi...@apache.org>.
fanyang closed pull request #16035: KAFKA-16257: Improve logical type compatibility in SchemaProjector
URL: https://github.com/apache/kafka/pull/16035
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16257: Improve logical type compatibility in SchemaProjector [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #16035:
URL: https://github.com/apache/kafka/pull/16035#issuecomment-2130356686
Hi @fanyang Thank you for contributing to Kafka! Is this something that seemed interesting to you, or do you have a use-case in mind?
I labeled this ticket as requires-kip, and if you're interested in tackling the larger project I'm happy to help. If you were just looking for an introductory ticket, check out the `newbie` label in Jira: https://issues.apache.org/jira/browse/KAFKA-15283?jql=project%20%3D%20KAFKA%20AND%20resolution%20%3D%20Unresolved%20AND%20labels%20%3D%20newbie%20ORDER%20BY%20priority%20DESC%2C%20updated%20DESC .
When I wrote the ticket originally, it was motivated by the three related tickets, and I included some of the motivation in a comment on one of the tickets: https://issues.apache.org/jira/browse/KAFKA-13505?focusedCommentId=17817494&page=com.atlassian.jira.plugin.system.issuetabpanels%3Acomment-tabpanel#comment-17817494
I apologize that I did not adequately summarize that comment in the description of KAFKA-16257, as I think I have caused some confusion.
Connect has "primitive" types, and "logical" types. Logical types are just primitive types plus some schema metadata defining how the primitive value should be interpreted. Connect includes a few standardized primitive types in the API package (Decimal, Date, Time, and Timestamp). Where the confusion arises is that these are not the only logical types out there. Any plugin can define a new logical type at any time using SchemaBuilder. These are the types that aren't able to safely traverse the SchemaProjector, and which motivated the ticket.
While the Decimal and Connect logical types aren't explicitly represented in the SchemaProjector implementation, they all use the no-op conversion path: https://github.com/apache/kafka/blob/d585a494a4871eaecdddf98f8655b8f30c5bd834/connect/api/src/main/java/org/apache/kafka/connect/data/SchemaProjector.java#L188
Regarding the conversion among the time types: A case could be made for Date -> Timestamp referring to the beginning of the day, but that assumes the intent of the user. But Time -> Timestamp interpreted as "Time t in 1970" is mostly nonsensical. I think both cases are abuses of how we represent these values using java.util.Date, and aren't actually desirable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16257: Improve logical type compatibility in SchemaProjector [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #16035:
URL: https://github.com/apache/kafka/pull/16035#issuecomment-2135540100
@fanyang Yes I think you understand. Your example is exactly the situation that needs improvement here.
Something similar to the Projectable interface is necessary, but I don't know the exact details yet, as we will need to work it out. There are also some classloading concerns about a Projectable implementation being in a different classloader than the plugin using SchemaProjector.
For example, if plugin A had the AvroConverter and the AvroEnumProjectable, and plugin B called SchemaProjector.project(..., avroEnum, ...), how would the SchemaProjector safely find and execute the AvroEnumProjectable?
You can think about these things and put together a proposal. I'm happy to answer any questions that you have. You can take a look at some of the isolation code https://github.com/apache/kafka/tree/trunk/connect/runtime/src/main/java/org/apache/kafka/connect/runtime/isolation and read the most relevant KIPs: https://cwiki.apache.org/confluence/display/KAFKA/KIP-146+-+Classloading+Isolation+in+Connect and https://cwiki.apache.org/confluence/display/KAFKA/KIP-898%3A+Modernize+Connect+plugin+discovery
> We can close it.
I agree.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16257: Improve logical type compatibility in SchemaProjector [kafka]
Posted by "fanyang (via GitHub)" <gi...@apache.org>.
fanyang commented on PR #16035:
URL: https://github.com/apache/kafka/pull/16035#issuecomment-2132884096
Hi Greg @gharris1727 ,
Thank you for the comments. I'm interest in this item, and my current usage scenario is also related to avro schema and connect schema. For example, when we write kafka connect record into avro file, we set avro file schema to the schema of the first record, and following records need to be compatible with the avro file.
I have read the comments of other three jira tickets. I think I got your point. For example, the SchemaProjector need to have following behavior:
```
// ConnectEnum is a logical type for enum defined in plugin
Schema sourceSchema = ConnectEnum.builder("Card", Arrays.asList("VISA", "MASTERCARD")).build();
Schema targetSchema = ConnectEnum.builder("Card", Arrays.asList("VISA", "MASTERCARD", "AMEX")).build();
// If the Compatibility is Backward, here shoudn't throw exception
Object projected = SchemaProjector.project(sourceSchema, Card.VISA, targetSchema);
```
My proposal is to add an interface like:
interface Projectable {
void checkCompatible(Schema source, Schema target, Compatibility compatibility)
}
and a schema class like:
class ProjectableConnectSchema extends ConnectSchema implements Projectable
and then, SchemaProjector can call Logical schemas to check their compatibility
What do you think?
I would like to start a KIP about this improvement. If you can help me with it, it's my honor.
For this PR. I agree with you about the Date/Time/Timestamp convert. So, the PR is not needed. We can close it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org