[ 
https://issues.apache.org/jira/browse/AVRO-1340?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16440169#comment-16440169
 ] 

ASF GitHub Bot commented on AVRO-1340:
--------------------------------------

cutting commented on issue #298: AVRO-1340: Added Enum Defaults and unit tests.
URL: https://github.com/apache/avro/pull/298#issuecomment-381783583
 
 
   Here's a fix for the IDL problem.  The grammar change seems to cause the 
parser to lookahead farther, so the last documentation comment that's been seen 
is too far along.  I changed it to grab the doc string once it's seen the enum 
keyword and that seems to fix it.
   ```--- 
a/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
   +++ b/lang/java/compiler/src/main/javacc/org/apache/avro/compiler/idl/idl.jj
   @@ -1102,12 +1102,12 @@ Schema EnumDeclaration():
      String defaultSymbol = null;
    }
    {
   -  "enum"
   +  "enum" {   String doc = getDoc(); }
      name = Identifier()
      symbols = EnumBody()
          [ <EQUALS> defaultSymbol=Identifier() <SEMICOLON>]
      {
   -    Schema s = Schema.createEnum(name, getDoc(), this.namespace, symbols,
   +    Schema s = Schema.createEnum(name, doc, this.namespace, symbols,
                                     defaultSymbol);
        names.put(s.getFullName(), s);
        return s;
   ```
   Patch otherwise looks good.  We should also test the IDL change.  Maybe 
change simple.avdl to specify an enum default, then update simple.avpr to 
contain the new output?  (You can use 'mvn package -dskipTests' to create a 
tools.jar so 'java -jar tools.jar idl ...' can be used to create the new 
compiler test output file.)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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


> use default to allow old readers to specify default enum value when 
> encountering new enum symbols
> -------------------------------------------------------------------------------------------------
>
>                 Key: AVRO-1340
>                 URL: https://issues.apache.org/jira/browse/AVRO-1340
>             Project: Avro
>          Issue Type: Improvement
>          Components: spec
>         Environment: N/A
>            Reporter: Jim Donofrio
>            Priority: Minor
>
> The schema resolution page says:
> > if both are enums:
> > if the writer's symbol is not present in the reader's enum, then an
> error is signalled.
> This makes it difficult to use enum's because you can never add a enum value 
> and keep old reader's compatible. Why not use the default option to refer to 
> one of enum values so that when a old reader encounters a enum ordinal it 
> does not recognize, it can default to the optional schema provided one. If 
> the old schema does not provide a default then the older reader can continue 
> to fail as it does today.



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

Reply via email to