-
Notifications
You must be signed in to change notification settings - Fork 344
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[feat][schema] Support the schema type ProtoNativeSchema #1006
Conversation
Could you fix lint error? You can use |
6443073
to
7c8a10f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! I left some comments.
@nodece Could you take a look again? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Motivation
The pulsar-client-go supports ProtoSchema based on the Avro format, users must provide the schema definition string when using struct schema(JSON, Avro, Proto), it's hard to use and we can't get the entire original proto definition.
The difference between ProtoSchema and ProtoNativeSchema is the schema data, the ProtoNativeSchema will contain the entire original proto definition, and we can use the schema data of the ProtoNativeSchema to generate the corresponding Descriptor and get any field, we can leverage this feature in Pulsar SQL.
The schema data of the ProtoNativeSchema follow the Pulsar format, we can generate JSON data for
ProtobufNativeSchemaData
as the schema data.The field
fileDescriptorSet
is the serialized data of the messageFileDescriptorSet
, it contains all definition proto files data.Modifications
Add a new schema type
ProtoNative(20)
.Add ProtoNativeSchema.
Add encode and decode methods for ProtoNativeSchema.
Verifying this change
Add an integration test to make sure the broker can deserialize the schema data of the ProtoNativeSchema.
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation