[Edit] Sorry, it’s documented here: https://pulsar.apache.org/docs/2.11.x/schema-understand/#schema-compatibility-check
From: Kiryl Valkovich <kiryl_valkovich@teal.tools> Date: Monday, February 20, 2023 at 3:48 PM To: dev@pulsar.apache.org <dev@pulsar.apache.org> Subject: Re: [DISCUSS] PIP-246: Improved PROTOBUF_NATIVE schema compatibility checks without using avro-protobuf Hm. I tend to think that for my Pulsar use case, it would be good to have the ability to implement a custom schema compatibility checker. For example, buf.build (a popular Protobuf linter and build) offers the following list of breaking rules. Half of them prefixed with FIELD_SAME_XXLANG_PACKAGE_ aren’t relevant. And I would want to use a subset of them if we are checking a single message descriptor. Most likely: ENUM_VALUE_NO_DELETE FIELD_NO_DELETE FIELD_SAME_TYPE ONEOF_NO_DELETE FIELD_SAME_LABEL RESERVED_ENUM_NO_DELETE RESERVED_MESSAGE_NO_DELETE I found out that the Pulsar broker has the following configuration option: schemaRegistryCompatibilityCheckers [org.apache.pulsar.broker.service.schema.JsonSchemaCompatibilityCheck, org.apache.pulsar.broker.service.schema.AvroSchemaCompatibilityCheck, org.apache.pulsar.broker.service.schema.ProtobufNativeSchemaCompatibilityCheck] But I can’t find documentation for it. At first look, it seems that for my need it would be enough to have a such custom configurable Protobuf message descriptor checker class that implements SchemaCompatibilityCheck. https://github.com/apache/pulsar/blob/82237d3684fe506bcb6426b3b23f413422e6e4fb/pulsar-broker/src/main/java/org/apache/pulsar/broker/service/schema/ProtobufNativeSchemaCompatibilityCheck.java#L31 [cid:image002.png@01D94541.991807E0] [Text Description automatically generated] From: SiNan Liu <liusinan1...@gmail.com> Date: Monday, February 20, 2023 at 1:41 PM To: dev@pulsar.apache.org <dev@pulsar.apache.org> Subject: Re: [DISCUSS] PIP-246: Improved PROTOBUF_NATIVE schema compatibility checks without using avro-protobuf It seems that we should only warn the user about changes to the field type, not define a compatibility check, or throw an exception. *Just like this: * *log.warn("proto.read.ProtobufSchema.uint64Field field type for uint64, was changed into a uint32");* I will update this in the PIP issue Alternatives and discuss both designs with other people. Kiryl Valkovich <kiryl_valkovich@teal.tools> 于2023年2月20日周一 18:14写道： > 1. Sure, I didn’t mean that it's only about the required fields. > 2. I found the page you are referring to. > https://protobuf.dev/programming-guides/proto3/#updating > > QUOTE START > If a number is parsed from the wire which doesn’t fit in the corresponding > type, you will get the same effect as if you had cast the number to that > type in C++ (for example, if a 64-bit number is read as an int32, it will > be truncated to 32 bits). > QUOTE END > > It’s discussable. Maybe I’m just missing something. > > I personally wouldn’t rely on such compatibility guarantees in a real > application. > If my check amount (> int32 lol) would be truncated because someone > changed the field type in a schema, I would be quite upset. > > From: SiNan Liu <liusinan1...@gmail.com> > Date: Monday, February 20, 2023 at 2:30 AM > To: dev@pulsar.apache.org <dev@pulsar.apache.org> > Subject: Re: [DISCUSS] PIP-246: Improved PROTOBUF_NATIVE schema > compatibility checks without using avro-protobuf > Thank you for your suggestions and questions. > 1. Your first question. It's not just a matter of the required field. There > should be many differences between proto3 and proto2. I will later test the > current code for compatibility checks in proto3, and also look at > compatibility between different protobuf versions. > 2. On your second question. This is the compatibility rule for field type > changes I found on the official website. You mean that this rule should not > pass the compatibility check. Or should an exception be thrown whenever the > field type changes? > > > Kiryl Valkovich <kiryl_valkovich@teal.tools> 于 2023年2月20日周一 上午12:31写道： > > > First, thank you for looking into it! > > > > There is a thing caught my eye: > > > > > The writtenSchema cannot add required fields, … > > > > As far as I know, the required fields were removed in Protocol Buffers > v3. > > > > I see proto3 usage in existing schema registry tests: > > > > > https://github.com/apache/pulsar/blob/6704f12104219611164aa2bb5bbdfc929613f1bf/pulsar-broker/src/test/proto/ProtobufSchemaTest.proto#L19 > > > > > > > https://github.com/apache/pulsar/blob/1ea4ad814f5f30b8c371db2a86626cd568ace553/pulsar-sql/presto-pulsar/src/test/java/org/apache/pulsar/sql/presto/decoder/protobufnative/TestMsg.proto#L19 > > > > I see proto2 usage in the proposed changes: > > > > > > > https://github.com/apache/pulsar/pull/19566/files#diff-f2f7463a3df6dc6366f3ee3a416707e655f0d5b8d2ae623a3f05a209c1d6ec88R19 > > > > > > > https://github.com/apache/pulsar/pull/19566/files#diff-f2f7463a3df6dc6366f3ee3a416707e655f0d5b8d2ae623a3f05a209c1d6ec88R19 > > > > Also, I’m not sure about this: > > > > > int32, uint32, int64, uint64, and bool are all compatible �C this means > > you can change a field from one of these types to another without > breaking > > forwards- or backwards-compatibility. > > > > It leads to JS/PHP-like implicit conversions if I understand it right. > > > > From: SiNan Liu <liusinan1...@gmail.com> > > Date: Sunday, February 19, 2023 at 1:59 PM > > To: dev@pulsar.apache.org <dev@pulsar.apache.org> > > Subject: [DISCUSS] PIP-246: Improved PROTOBUF_NATIVE schema compatibility > > checks without using avro-protobuf > > Hi all, > > > > I made a PIP to discuss: https://github.com/apache/pulsar/issues/19565. > > > > We can talk about the current design here. Especially for the field type > > change check rules, please give your valuable advice. > > > > Thanks, > > Sinan > > >
