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

Avro 1.9+ dropping defaults #298

Open
majisourav99 opened this issue Mar 14, 2022 · 4 comments
Open

Avro 1.9+ dropping defaults #298

majisourav99 opened this issue Mar 14, 2022 · 4 comments

Comments

@majisourav99
Copy link
Collaborator

The helper builder drops the default value if the default value type does not match, eg if 0 is a default value of field type float then the following code will silently drop the default

public FieldBuilder19(Schema.Field other) {
if (other != null) {
_name = other.name();
_schema = other.schema();
_doc = other.doc();
_defaultVal = other.defaultVal(); // Its null for Avro 1.9+
_order = other.order(); // other.order() cannot be null under Avro 1.9.*
_props = other.getObjectProps();
}
}

@srramach
Copy link
Collaborator

Are you asking for an exception to be thrown, instead of silently dropping the default?

@majisourav99
Copy link
Collaborator Author

Using the avro-util AvroCompatibilityHelper default getter method, we should set the correct default value.

@srramach
Copy link
Collaborator

I think it's more correct to throw an exception. Silently turning invalid default values into valid ones seems like it would just mask issues that originate/happen elsewhere.

@radai-rosenblatt
Copy link
Contributor

there is a bug in avro 1.9 if int default values are provided for float fields. we should be able to work around it in helper code.
also worth having a spotbugs rule for use of the defaultVal() method perhaps as it is vulnerable to this bug in 1.9?

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

3 participants