[ 
https://issues.apache.org/jira/browse/AVRO-3374?focusedWorklogId=782034&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-782034
 ]

ASF GitHub Bot logged work on AVRO-3374:
----------------------------------------

                Author: ASF GitHub Bot
            Created on: 16/Jun/22 12:59
            Start Date: 16/Jun/22 12:59
    Worklog Time Spent: 10m 
      Work Description: opwvhk commented on code in PR #1688:
URL: https://github.com/apache/avro/pull/1688#discussion_r899055830


##########
lang/java/avro/src/main/java/org/apache/avro/Schema.java:
##########
@@ -751,8 +751,32 @@ public void writeName(Names names, JsonGenerator gen) 
throws IOException {
     }
 
     public String getQualified(String defaultSpace) {
-      return (space == null || space.equals(defaultSpace)) ? name : full;
+      return this.shouldWriteFull(defaultSpace) ? full : name;
     }
+
+    /**
+     * Determine if full name must be written. There are 2 cases for true :
+     * defaultSpace != from this.space or name is already a Schema.Type (int, 
array
+     * ...)
+     *
+     * @param defaultSpace : default name space.
+     * @return true if full name must be written.
+     */
+    private boolean shouldWriteFull(String defaultSpace) {
+      if (space != null && space.equals(defaultSpace)) {
+        try {
+          // name is a 'Type', so namespace must be written (int should return 
true, Int
+          // should return false).
+          return 
Type.valueOf(this.name.toUpperCase(Locale.ENGLISH)).name.equals(this.name);
+        } catch (IllegalArgumentException ex) {
+          // namespace can be omitted, (default & name is not a type)
+          return false;
+        }

Review Comment:
   This code catches the exception as flow control, which is quite expensive. I 
think we should prefer a comparison loop instead:
   ```
         for (Type type : Type.values()) {
           if (type.name.equals(name)) {
             return true;
           }
         }
         return false;
   ```





Issue Time Tracking
-------------------

    Worklog Id:     (was: 782034)
    Time Spent: 0.5h  (was: 20m)

> [Java] Fully qualified type reference "ns.int" loses namespace.
> ---------------------------------------------------------------
>
>                 Key: AVRO-3374
>                 URL: https://issues.apache.org/jira/browse/AVRO-3374
>             Project: Apache Avro
>          Issue Type: Bug
>          Components: java
>    Affects Versions: 1.11.0
>            Reporter: Ryan Skraba
>            Assignee: Christophe Le Saec
>            Priority: Minor
>              Labels: pull-request-available, pull-requests-available
>             Fix For: 1.12.0
>
>         Attachments: AVRO-3374.patch
>
>          Time Spent: 0.5h
>  Remaining Estimate: 0h
>
> While brainstorming for AVRO-3370, I came across this special case where a 
> type-reference could be considered ambiguous if the SDK is not careful when 
> simplifying inherited namespaces:
> {code:json}
> {
>   "type" : "record",
>   "name" : "ns.int",
>   "fields" : [ 
>     {"name" : "value", "type" : "int"}, 
>     {"name" : "next", "type" : [ "null", "ns.int" ]}
>   ]
> }
> {code}
> In Java, if this code is parsed, it works as expected (as a linked list).
> If the schema is turned to a String using toString(), the namespace is 
> dropped off the last {*}{{ns.int}}{*}, turning it into the primitive. That 
> string can still be parsed into a Schema, but the "round-trip" modifies the 
> schema in an incompatible way.
> That namespace shouldn't be dropped when producing the JSON string 
> representing the Schema in Java.



--
This message was sent by Atlassian Jira
(v8.20.7#820007)

Reply via email to