-
Notifications
You must be signed in to change notification settings - Fork 3.5k
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
[Issue #10427] Add AvroSchema UUID support fix #10428
Conversation
b5ec4f4
to
fbe13be
Compare
pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/AvroSchemaTest.java
Show resolved
Hide resolved
pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/AvroSchemaTest.java
Outdated
Show resolved
Hide resolved
ea763d0
to
27cfead
Compare
pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/AvroSchema.java
Outdated
Show resolved
Hide resolved
Because the issue happens when creating the AVRO schema from a POJO class. Maybe a java annotation on BigDecimal fields could provide the scale ?
… Le 30 avr. 2021 à 15:38, Enrico Olivelli ***@***.***> a écrit :
@eolivelli commented on this pull request.
In pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/AvroSchema.java:
> @@ -127,7 +127,12 @@ public static void addLogicalTypeConversions(ReflectData reflectData, boolean js
}
}
reflectData.addLogicalTypeConversion(new Conversions.UUIDConversion());
- reflectData.addLogicalTypeConversion(new Conversions.DecimalConversion());
+ reflectData.addLogicalTypeConversion(new Conversions.DecimalConversion() {
+ @OverRide
+ public org.apache.avro.Schema getRecommendedSchema() {
+ return LogicalTypes.decimal(100,51).addToSchema(org.apache.avro.Schema.create(org.apache.avro.Schema.Type.BYTES));
we already have "isJsr310ConversionEnabled" and "getAlwaysAllowNull"
why don't we store precision and scale the same way ? this way they will be stored in the registry and consumers will use the right version of the schema
—
You are receiving this because you were assigned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
a803d21
to
9169403
Compare
pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/AvroSchemaTest.java
Outdated
Show resolved
Hide resolved
caa75c1
to
f61e75b
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.
LGTM
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
pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/util/SchemaUtil.java
Show resolved
Hide resolved
pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/util/SchemaUtil.java
Show resolved
Hide resolved
e17d00a
to
29113ee
Compare
29113ee
to
8483966
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.
@sijie I believe that now the patch is really good to go.
please take another look when you have time
@sijie I believe that now the patch is really good to go. please take another look when you have time |
### Motivation I can use Avro schema with BigDecimal in Pulsar 2.8.0, but this doesn't work on Pulsar 2.8.1, so I check this codebase and PR, found #10428 breaks this. The following is error log: ``` org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:154) ~[spring-beans-5.3.4.jar:5.3.4] ... 34 common frames omitted Caused by: java.lang.UnsupportedOperationException: No recommended schema for decimal (scale is required) at org.apache.pulsar.shade.org.apache.avro.Conversions$DecimalConversion.getRecommendedSchema(Conversions.java:73) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createSchema(ReflectData.java:696) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createFieldSchema(ReflectData.java:873) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData$AllowNull.createFieldSchema(ReflectData.java:92) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createSchema(ReflectData.java:736) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:328) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:325) ~[pulsar-client-2.8.1.jar:2.8.1] at java.base/java.lang.ClassValue.getFromHashMap(ClassValue.java:228) ~[na:na] at java.base/java.lang.ClassValue.getFromBackup(ClassValue.java:210) ~[na:na] at java.base/java.lang.ClassValue.get(ClassValue.java:116) ~[na:na] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData.getSchema(SpecificData.java:339) ~[pulsar-client-2.8.1.jar:2.8.1] ... 52 common frames omitted ``` I think that Avro cannot work on using the ReflectData with Conversions.DecimalConversion to parse the BigDecimal field without AvroSchema when getting schema, but the Avro ReflectDatumWriter and ReflectDatumReader are working, it seems that Avro support for BigDecimal is not enough. Affected version: 2.8.1...2.8.x, 2.9.x ### Modifications - Skip add the DecimalConversion in `extractAvroSchema()`
### Motivation I can use Avro schema with BigDecimal in Pulsar 2.8.0, but this doesn't work on Pulsar 2.8.1, so I check this codebase and PR, found #10428 breaks this. The following is error log: ``` org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:154) ~[spring-beans-5.3.4.jar:5.3.4] ... 34 common frames omitted Caused by: java.lang.UnsupportedOperationException: No recommended schema for decimal (scale is required) at org.apache.pulsar.shade.org.apache.avro.Conversions$DecimalConversion.getRecommendedSchema(Conversions.java:73) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createSchema(ReflectData.java:696) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createFieldSchema(ReflectData.java:873) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData$AllowNull.createFieldSchema(ReflectData.java:92) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createSchema(ReflectData.java:736) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:328) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:325) ~[pulsar-client-2.8.1.jar:2.8.1] at java.base/java.lang.ClassValue.getFromHashMap(ClassValue.java:228) ~[na:na] at java.base/java.lang.ClassValue.getFromBackup(ClassValue.java:210) ~[na:na] at java.base/java.lang.ClassValue.get(ClassValue.java:116) ~[na:na] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData.getSchema(SpecificData.java:339) ~[pulsar-client-2.8.1.jar:2.8.1] ... 52 common frames omitted ``` I think that Avro cannot work on using the ReflectData with Conversions.DecimalConversion to parse the BigDecimal field without AvroSchema when getting schema, but the Avro ReflectDatumWriter and ReflectDatumReader are working, it seems that Avro support for BigDecimal is not enough. Affected version: 2.8.1...2.8.x, 2.9.x ### Modifications - Skip add the DecimalConversion in `extractAvroSchema()` (cherry picked from commit 2ca8e8a)
### Motivation I can use Avro schema with BigDecimal in Pulsar 2.8.0, but this doesn't work on Pulsar 2.8.1, so I check this codebase and PR, found #10428 breaks this. The following is error log: ``` org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:154) ~[spring-beans-5.3.4.jar:5.3.4] ... 34 common frames omitted Caused by: java.lang.UnsupportedOperationException: No recommended schema for decimal (scale is required) at org.apache.pulsar.shade.org.apache.avro.Conversions$DecimalConversion.getRecommendedSchema(Conversions.java:73) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createSchema(ReflectData.java:696) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createFieldSchema(ReflectData.java:873) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData$AllowNull.createFieldSchema(ReflectData.java:92) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createSchema(ReflectData.java:736) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:328) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:325) ~[pulsar-client-2.8.1.jar:2.8.1] at java.base/java.lang.ClassValue.getFromHashMap(ClassValue.java:228) ~[na:na] at java.base/java.lang.ClassValue.getFromBackup(ClassValue.java:210) ~[na:na] at java.base/java.lang.ClassValue.get(ClassValue.java:116) ~[na:na] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData.getSchema(SpecificData.java:339) ~[pulsar-client-2.8.1.jar:2.8.1] ... 52 common frames omitted ``` I think that Avro cannot work on using the ReflectData with Conversions.DecimalConversion to parse the BigDecimal field without AvroSchema when getting schema, but the Avro ReflectDatumWriter and ReflectDatumReader are working, it seems that Avro support for BigDecimal is not enough. Affected version: 2.8.1...2.8.x, 2.9.x ### Modifications - Skip add the DecimalConversion in `extractAvroSchema()` (cherry picked from commit 2ca8e8a)
### Motivation I can use Avro schema with BigDecimal in Pulsar 2.8.0, but this doesn't work on Pulsar 2.8.1, so I check this codebase and PR, found apache#10428 breaks this. The following is error log: ``` org.springframework.beans.factory.support.SimpleInstantiationStrategy.instantiate(SimpleInstantiationStrategy.java:154) ~[spring-beans-5.3.4.jar:5.3.4] ... 34 common frames omitted Caused by: java.lang.UnsupportedOperationException: No recommended schema for decimal (scale is required) at org.apache.pulsar.shade.org.apache.avro.Conversions$DecimalConversion.getRecommendedSchema(Conversions.java:73) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createSchema(ReflectData.java:696) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createFieldSchema(ReflectData.java:873) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData$AllowNull.createFieldSchema(ReflectData.java:92) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.reflect.ReflectData.createSchema(ReflectData.java:736) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:328) ~[pulsar-client-2.8.1.jar:2.8.1] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData$3.computeValue(SpecificData.java:325) ~[pulsar-client-2.8.1.jar:2.8.1] at java.base/java.lang.ClassValue.getFromHashMap(ClassValue.java:228) ~[na:na] at java.base/java.lang.ClassValue.getFromBackup(ClassValue.java:210) ~[na:na] at java.base/java.lang.ClassValue.get(ClassValue.java:116) ~[na:na] at org.apache.pulsar.shade.org.apache.avro.specific.SpecificData.getSchema(SpecificData.java:339) ~[pulsar-client-2.8.1.jar:2.8.1] ... 52 common frames omitted ``` I think that Avro cannot work on using the ReflectData with Conversions.DecimalConversion to parse the BigDecimal field without AvroSchema when getting schema, but the Avro ReflectDatumWriter and ReflectDatumReader are working, it seems that Avro support for BigDecimal is not enough. Affected version: 2.8.1...2.8.x, 2.9.x ### Modifications - Skip add the DecimalConversion in `extractAvroSchema()`
Fixes #10427
Motivation
As AVRO support the UUID type (see AVRO spec), add UUID support for the pulsar AvroSchema.
It also adds the AVRO Decimal logical type (see AVRO spec).
Modifications
Add the logical type conversion to the ReflectData instance when parsing the provided POJO.
Verifying this change
See the provided unit test in pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/AvroSchemaTest.java