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

Cannot set default value for timestamp logical types #122

Closed
jameshartig opened this issue Aug 27, 2018 · 4 comments
Closed

Cannot set default value for timestamp logical types #122

jameshartig opened this issue Aug 27, 2018 · 4 comments

Comments

@jameshartig
Copy link

The MarshalJSON method for the time.Time struct:

// MarshalJSON implements the json.Marshaler interface.
// The time is a quoted string in RFC 3339 format, with sub-second precision added if present.

Which means if you're generating an avro file and specify a default of time.Time{} it will marshal that as a string and then you get:

cannot create OCFWriter: cannot create OCF header: Record "analytics.submission" field "ts": default value ought to encode using field schema: cannot transform binary timestamp-micros, expected time.Time, received string

Since NewCodec unmarshalling the schema into an interface{} there's no way for the json package to know that it should be a time.Time to unmarshal into and not a plain string. I think timeStampMillisFromNative (and other timestamp types) should check to see if the value is a string and then unmarshal into a time.Time and that should fix the issue.

@kdvy, what do you think?

@kdvy
Copy link
Contributor

kdvy commented Aug 27, 2018

Apologies for the oversight! We didn't have a requirement for default values and it slipped my mind.

I certainly have no objections to your suggestion and it sounds like the correct approach to me. I'm afraid I don't to have time to add this functionality at the moment due to impending holiday + deadlines.

Thanks for pointing this out and feel free to ping me if you need anything.

@kdvy
Copy link
Contributor

kdvy commented Oct 20, 2018

Have created a PR, but a schema that uses defaults with RFC3339 may not be cross-library compatible since the base type is long.

@karrick
Copy link
Contributor

karrick commented Jun 21, 2019

Resolved by #136

@kdarkhan
Copy link

kdarkhan commented Nov 6, 2019

@karrick this issue is not resolved because the linked PR was rejected.
Can we reopen this?

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

No branches or pull requests

4 participants