merlimat commented on a change in pull request #3752: revise the schema default 
type not null
URL: https://github.com/apache/pulsar/pull/3752#discussion_r264984110
 
 

 ##########
 File path: 
pulsar-client-api/src/main/java/org/apache/pulsar/client/api/Schema.java
 ##########
 @@ -167,25 +168,47 @@ default T decode(byte[] bytes, byte[] schemaVersion) {
     }
 
     /**
-     * Create a Avro schema type by extracting the fields of the specified 
class.
+     * Create a  Avro schema type by default configuration of the class
      *
      * @param clazz the POJO class to be used to extract the Avro schema
      * @return a Schema instance
      */
     static <T> Schema<T> AVRO(Class<T> clazz) {
-        return DefaultImplementation.newAvroSchema(clazz);
+        return DefaultImplementation.newAvroSchema(new 
SchemaDefinition<>(clazz));
     }
 
     /**
-     * Create a JSON schema type by extracting the fields of the specified 
class.
+     * Create a Avro schema type with schema definition
+     *
+     * @param schemaDefinition the definition of the schema
+     * @return a Schema instance
+     */
+    static <T> Schema<T> AVRO(SchemaDefinition<T> schemaDefinition) {
+        return DefaultImplementation.newAvroSchema(schemaDefinition);
+    }
+
+
+    /**
+     * Create a JSON schema type by default configuration of the class
      *
      * @param clazz the POJO class to be used to extract the JSON schema
      * @return a Schema instance
      */
     static <T> Schema<T> JSON(Class<T> clazz) {
-        return DefaultImplementation.newJSONSchema(clazz);
+        return DefaultImplementation.newJSONSchema(new 
SchemaDefinition<>(clazz));
+    }
+
+    /**
+     * Create a JSON schema type with schema definition
+     *
+     * @param schemaDefinition the definition of the schema
+     * @return a Schema instance
+     */
+    static <T> Schema<T> JSON(SchemaDefinition schemaDefinition) {
 
 Review comment:
   A part from having this difference in the allow null, I think there is a 
more fundamental difference between JSON and AVRO.
   
   In JSON, the AVRO schema is extracted and used for the validation, but the 
JSON serialization is done separately. In this case the POJO and the serialized 
data could be different from the Avro schema that was validated with broker.
   
   That's not a problem with Avro since the schema object is used at 
serialization time so it doesn't matter which POJO we're passing there that the 
result will be correct (or user gets exception). 
   
   For this I'm suggesting to avoid adding the option for JSON right now and 
defer it until there's a clear use case for it (adding is always easy).
   
   My guess is that this will probably not be needed as users will not already 
have a precise Avro schema representation for the data used in other systems. 
Rather, it's common practice in most languages to define POJOs for the JSON 
data.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to