[GitHub] [avro] opwvhk commented on a diff in pull request #1970: Avro-3313: [Java] use default enum when reading with old schema

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

opwvhk commented on code in PR #1970:
URL: https://github.com/apache/avro/pull/1970#discussion_r1033171936


##########
lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java:
##########
@@ -265,7 +267,17 @@ protected void readField(Object record, Field field, Object oldDatum, ResolvingD
    * representations. By default, returns a GenericEnumSymbol.
    */
   protected Object readEnum(Schema expected, Decoder in) throws IOException {
-    return createEnum(expected.getEnumSymbols().get(in.readEnum()), expected);
+    List<String> enumSymbols = expected.getEnumSymbols();
+    int ordinal = in.readEnum();
+    if (ordinal >= enumSymbols.size()) {
+      if (expected.getEnumDefault() == null) {
+        throw new AvroTypeException("Unknown Enum Ordinal " + ordinal);
+      }
+
+      return createEnum(expected.getEnumDefault(), expected);

Review Comment:
   Without a (correct!) writer schema, Avro data is basically garbage. Attempting to fix this is a laudible effort, but also introduces bugs.
   
   What happens, for example if data was written with `{"type":"enum","name":"Switch","symbols":["OFF","DIMMED1","DIMMED2","DIMMED3","FULL"]}`, but subsequently read with the older `{"type":"enum","name":"Switch","symbols":["ON","OFF"]}`?
   
   IMHO, throwing an exception is the correct way to handle this. 



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@avro.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org