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] Basic logicalType support for date time types #278

Conversation

MichalFoksa
Copy link
Contributor

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:

  • java.util.Date
  • java.time.OffsetDateTime
  • java.time.ZonedDateTime
  • java.time.Instant
  • java.time.LocalDate
  • java.time.LocalTime
  • java.time.LocalDateTime

@MichalFoksa MichalFoksa changed the title Basic logicalType support for date time types [Avro] Basic logicalType support for date time types May 16, 2021
@cowtowncoder
Copy link
Member

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
Copy link
Contributor Author

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

@MichalFoksa MichalFoksa force-pushed the feature/avro/basic_logicalType_support_for_date_time_types branch from b36af7b to e16412c Compare May 16, 2021 17:48
@MichalFoksa MichalFoksa changed the base branch from master to 2.13 May 16, 2021 17:48
@MichalFoksa MichalFoksa force-pushed the feature/avro/basic_logicalType_support_for_date_time_types branch from e16412c to 56c940b Compare May 16, 2021 17:53
@MichalFoksa MichalFoksa changed the base branch from 2.13 to 2.12 May 16, 2021 17:54
@MichalFoksa
Copy link
Contributor Author

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

@MichalFoksa
Copy link
Contributor Author

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
Copy link
Member

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 MichalFoksa force-pushed the feature/avro/basic_logicalType_support_for_date_time_types branch from 56c940b to ff76797 Compare May 16, 2021 18:02
@MichalFoksa MichalFoksa changed the base branch from 2.12 to 2.13 May 16, 2021 18:02
@MichalFoksa
Copy link
Contributor Author

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
Copy link
Member

@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
Copy link
Contributor Author

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

@MichalFoksa MichalFoksa force-pushed the feature/avro/basic_logicalType_support_for_date_time_types branch from ff76797 to 3b0037f Compare May 26, 2021 18:31
@MichalFoksa MichalFoksa marked this pull request as draft June 6, 2021 07:18
@MichalFoksa MichalFoksa force-pushed the feature/avro/basic_logicalType_support_for_date_time_types branch from 61d2315 to 491ab5b Compare June 6, 2021 07:29
@MichalFoksa MichalFoksa force-pushed the feature/avro/basic_logicalType_support_for_date_time_types branch from 27955ce to 0e0d4d5 Compare June 6, 2021 13:50
…edDateTime to Avro long type and logicalType.
@MichalFoksa MichalFoksa force-pushed the feature/avro/basic_logicalType_support_for_date_time_types branch from 0e0d4d5 to e23d0b4 Compare June 6, 2021 21:02
@MichalFoksa
Copy link
Contributor Author

I will open new PR when I am ready.

@MichalFoksa MichalFoksa closed this Jun 7, 2021
@cowtowncoder
Copy link
Member

@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
Copy link
Contributor Author

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

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

Successfully merging this pull request may close these issues.

2 participants