Skip to content

NPEs thrown by serialization for mandatory fields could have better error messages #553

@FelixGV

Description

@FelixGV

When serializing a record with mandatory fields, there is no null check. For a mandatory string field, fast-avro generates code like this:

if (((CharSequence) data.get(1)) instanceof Utf8) {
  (encoder).writeString(((Utf8)((CharSequence) data.get(1))));
} else {
  (encoder).writeString(((CharSequence) data.get(1)).toString());
}

Line 4 will throw a NPE due to calling toString() on the null field.

Throwing a NPE is not necessarily a problem in and of itself (and we likely need to preserve that behavior in order to be compatible with vanilla Avro) but a better error message would be welcome. For example, the generated code might instead look like:

Object value = data.get(1);
if (value instanceof Utf8) {
  (encoder).writeString((Utf8) value);
} else if (value == null) {
  throw new NullPointerException("Field fieldName is not allowed to be null");
} else {
  (encoder).writeString(value.toString());
}

This adds one more check on the hot path, though in the end the VM must do this check anyway when dereferencing, so it might not matter in terms of bytecode/JIT.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions