Closed kucera-jan-cz closed 5 years ago
@kucera-jan-cz Apologies for slow follow up. Hope to review soon now, finally.
One request: could this be rebased (or new PR created) for 2.10? master
targets 3.0.0, and that's quite a ways away: so it'd be great to get this in 2.10 instead (from which I'll merge to master
).
Ok: so, aside from one comment on Javadoc for the new feature (need to indicate exactly where it applies), I have one other question -- does this check that field type allows null
s? In Avro null is a distinct type so default value of null
is only legal for union type that include null type.
does this check that field type allows nulls?
Yes-- My comment was on the conditional that makes this check; Specifically, if:
@AvroDefault()
or @JsonProperty(defaultValue=)
)UNION
and NULL
is a valid type in that unionthen set the default value to null
.
Hmmh. Ok, I think I'll merge this in and change naming.
Looks like there's one snag wrt 2.10, so will first just merge to master
.
Added: new Feature enum AVRO_DEFAULT_ENABLED for enabling null inclusion RecordVisitor is now include NullNode if AVRO_DEFAULT_ENABLED is enabled and field is not primitive