[GitHub] [pulsar] eolivelli commented on a change in pull request #10527: cleaned some code in GenericJsonRecord

[GitBox <gi...@apache.org>]

eolivelli commented on a change in pull request #10527:
URL: https://github.com/apache/pulsar/pull/10527#discussion_r630011492



##########
File path: pulsar-client/src/main/java/org/apache/pulsar/client/impl/schema/generic/GenericJsonRecord.java
##########
@@ -102,29 +102,26 @@ public Object getField(String fieldName) {
     }
 
     private boolean isBinaryValue(String fieldName) {
-        boolean isBinary = false;
-
-        do {
-            if (schemaInfo == null) {
-                break;
-            }
+        if (schemaInfo == null) {
+            return false;
+        }
 
-            try {
-                org.apache.avro.Schema schema = parseAvroSchema(schemaInfo.getSchemaDefinition());
-                org.apache.avro.Schema.Field field = schema.getField(fieldName);
-                ObjectMapper objectMapper = new ObjectMapper();
-                JsonNode jsonNode = objectMapper.readTree(field.schema().toString());
-                for (JsonNode node : jsonNode) {
-                    JsonNode jn = node.get("type");
-                    if (jn != null && ("bytes".equals(jn.asText()) || "byte".equals(jn.asText()))) {
-                        isBinary = true;
-                    }
+        boolean isBinary = false;
+        try {
+            org.apache.avro.Schema schema = parseAvroSchema(schemaInfo.getSchemaDefinition());
+            org.apache.avro.Schema.Field field = schema.getField(fieldName);
+            ObjectMapper objectMapper = new ObjectMapper();

Review comment:
       If field is null then we have a NPE, can you fix it please?
   It is not strictly related to this patch but I believe it is worth to do




-- 
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:
users@infra.apache.org