Skip to content
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

[Bug] Upload BYTES schema but got NULL schema type #18825

Closed
1 of 2 tasks
codelipenghui opened this issue Dec 8, 2022 · 5 comments · Fixed by #18995
Closed
1 of 2 tasks

[Bug] Upload BYTES schema but got NULL schema type #18825

codelipenghui opened this issue Dec 8, 2022 · 5 comments · Fixed by #18995
Assignees
Labels
type/bug The PR fixed a bug or issue reported a bug

Comments

@codelipenghui
Copy link
Contributor

Search before asking

  • I searched in the issues and found nothing similar.

Version

Latest master

Minimal reproduce step

root@stg-billing-broker-0:/pulsar# cat schema.txt
{
"type": "BYTES",
"schema": "",
"properties": {}
}

root@stg-billing-broker-0:/pulsar# ./bin/pulsar-admin schemas upload persistent://public/default/asher-test -f schema.txt

root@stg-billing-broker-0:/pulsar# ./bin/pulsar-admin schemas get persistent://public/default/asher-test
{
"version": 4,
"schemaInfo": {
"name": "asher-test",
"schema": "",
"type": "NONE",
"properties": {}
}
}

What did you expect to see?

To keep consistent with the client side.
I think we should prevent the BYTES schema uploading
It's need to have discussion under the mailing list since it will change the existing behavior

What did you see instead?

No

Anything else?

No

Are you willing to submit a PR?

  • I'm willing to submit a PR!
@labuladong
Copy link
Contributor

Assign to me please

@codelipenghui
Copy link
Contributor Author

Thanks @labuladong

@labuladong
Copy link
Contributor

The root cause is BYTES is a negative enum:

//
// Schemas that don't have schema info. the value should be negative.
//
/**
* A bytes array.
*/
BYTES(-1),

Then convertFromDomainType convert BYTES to NONE

static SchemaRegistryFormat.SchemaInfo.SchemaType convertFromDomainType(SchemaType type) {
if (type.getValue() < 0) {
return SchemaRegistryFormat.SchemaInfo.SchemaType.NONE;
} else {
return SchemaRegistryFormat.SchemaInfo.SchemaType.valueOf(type.getValue() + 1);
}
}

@labuladong
Copy link
Contributor

I think there are two solutions:

  1. Forbid users to upload the BYTES schema. This is the easiest way, don't need to change much code. (recommended)

I tried Java client Producer<byte[]> producer = client.newProducer(Schema.BYTES), this producer won't auto-create a BYTES schema. If we allow users to upload a BYTES schema, that may cause inconsistency.

  1. Add a BYTES type schema, that needs to add BYTES type to proto file:

message SchemaInfo {
enum SchemaType {
NONE = 1;
STRING = 2;
JSON = 3;
PROTOBUF = 4;
AVRO = 5;
BOOLEAN = 6;
INT8 = 7;
INT16 = 8;
INT32 = 9;
INT64 = 10;
FLOAT = 11;
DOUBLE = 12;
DATE = 13;
TIME = 14;
TIMESTAMP = 15;
KEYVALUE = 16;
INSTANT = 17;
LOCALDATE = 18;
LOCALTIME = 19;
LOCALDATETIME = 20;
PROTOBUFNATIVE = 21;
}

And client.newProducer(Schema.BYTES) should also create BYTES type schema in schema factory.

@labuladong
Copy link
Contributor

congbobo184 pushed a commit that referenced this issue Dec 28, 2022
Fixes [#18825](#18825)

### Motivation

Admin API can upload `BYTES` schema and store a `NONE` schema in the schema registry, which is inconsistent with the client side. After discussion, we decided to forbid users from uploading `BYTES` schema.

Mail list: https://lists.apache.org/thread/672zmptfblwjmrf9z8336mk12r7csngf

### Modifications

Add a check to reject `BYTES` schema upload.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants