Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
FelixGV opened this issue Mar 11, 2024 · 0 comments

Comments

@FelixGV
Copy link
Collaborator

FelixGV commented Mar 11, 2024

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant