search

FasterXML / jackson-dataformats-binary

Uber-project for standard Jackson binary format backends: avro, cbor, ion, protobuf, smile
Apache License 2.0
316 stars 136 forks source link

[Avro] Basic logicalType support for date time types #278

Closed MichalFoksa closed 3 years ago

MichalFoksa commented 3 years ago

PR in reference to #277, point 1.

Simple mapping of few Java date-time types to Avro logicalType. The mapping works if ObjectMapper is used with JavaTimeModule and WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS is disabled. In this combination JSR310 types are mapped to Avro LONG, only the logical type is missing.

new ObjectMapper
    .registerModule(new JavaTimeModule())
    .disable(SerializationFeature.WRITE_DATE_TIMESTAMPS_AS_NANOSECONDS)
...

Supported java types:

cowtowncoder commented 3 years ago

I assume you would like this for 2.x? If so, this needs to be rebased against 2.13 branch; master is for 3.0 which is bit far from release candidate phase.

MichalFoksa commented 3 years ago

Yes, for 2.x please. I will rebase it.

MichalFoksa commented 3 years ago

@cowtowncoder I have rebased it onto 2.12 - is it OK?

MichalFoksa commented 3 years ago

Sorry, I read your comment too quickly. I will rebase it onto 2.13.

I assume you would like this for 2.x? If so, this needs to be rebased against 2.13 branch; master is for 3.0 which is bit far from release candidate phase.

cowtowncoder commented 3 years ago

Ok thanks! This seems like relatively safe change, but use of java.time types means Java 8 and officially Jackson 2.12 was still Java 7. Jackson 2.13 requires Java 8 (except for jackson-core/-annotations).

MichalFoksa commented 3 years ago

Sure, 2.13 it fine.

BTW: There is a failing test in 2.13. I cannot find cause quickly - there is no change in Avro module between 2.12 and 2.13 obviously realted to the error:

[ERROR] Errors:
[ERROR]   TestSimpleGeneration.testSchemaForUntypedMap:166 » UnsupportedOperation Maps w...
cowtowncoder commented 3 years ago

@MichalFoksa yes, I noticed that odd new failure just before leaving for vacation but haven't had time to see what gives. Hoping to look at it soon; definitely should not have failures from clean clone. :-(

MichalFoksa commented 3 years ago

hey :) look at here #281 - it issue about the problem with attached PR.

MichalFoksa commented 3 years ago

I will open new PR when I am ready.

cowtowncoder commented 3 years ago

@MichalFoksa Ok just let me know! I have been bit busy with non-Jackson things for past 2 weeks but will try to get things reviewed and merged. Plus as per

https://github.com/FasterXML/jackson/issues/83

now starting to focus on getting RC1 of 2.13.0 out soon -- there is still time for this and other changes but want to make sure I know the state of things.

MichalFoksa commented 3 years ago

@cowtowncoder Here you are new PR #283 - I think it is ready for review.