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-4043] Add time-nanos logical type #3125

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

glywk
Copy link
Contributor

@glywk glywk commented Aug 28, 2024

AVRO-4043

What is the purpose of the change

This pull request describes the time of day specification with a nano seconds precision as requested in AVRO-4043

Verifying this change

This change is a specification description.

Documentation

  • Does this pull request introduce a new feature? (yes)
  • If yes, how is the feature documented? (docs)

Copy link
Contributor

@opwvhk opwvhk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The consistency is good (minor nitpick: the anchor should no longer have the _ms suffix) . Thank you.

However, increasing the granularity of a timestamp a 1000/million-fold also reduces its range by the same factor. As such, I'd like us to document the possible maximum timestamp values as well.

@martin-g
Copy link
Member

Changes to the specification should be discussed at [email protected] first.

@glywk
Copy link
Contributor Author

glywk commented Aug 30, 2024

Changes to the specification should be discussed at [email protected] first.

Sorry, I ignore this aspect. How to register to [email protected] to submit this proposal?

@glywk
Copy link
Contributor Author

glywk commented Aug 30, 2024

The consistency is good (minor nitpick: the anchor should no longer have the _ms suffix) . Thank you.

However, increasing the granularity of a timestamp a 1000/million-fold also reduces its range by the same factor. As such, I'd like us to document the possible maximum timestamp values as well.

I have kept 'time_ms' link to preserve backward compatibility. But I will remove it.

@martin-g
Copy link
Member

How to register to [email protected] to submit this proposal?

You can click on "Subscribe to mailing list" at https://lists.apache.org/[email protected]
Or directly send a dummy email to [email protected] and then just Reply on the confirmation email.

@glywk
Copy link
Contributor Author

glywk commented Sep 1, 2024

Changes to the specification should be discussed at [email protected] first.

Hi @martin-g, I'll let you discuss among yourselves whether you want all time scales to support nanoseconds or not.
Time is just a timestamp subset. As request, I have also add timestamp limit for each scale. This PR can help you in your decision.

There is also the duration logical type but it involves a breaking change.

Regards

@martin-g
Copy link
Member

Hi @martin-g, I'll let you discuss among yourselves

There is no such thing! Everything is open at the ASF and should be discussed in the mailing lists where everyone could participate!
I have no personal interest in this feature, so I won't lead a discussion. If you need it then you need to explain why it is useful and and convince others that the specification needs to be updated.

@martin-g martin-g marked this pull request as draft September 12, 2024 08:30
@martin-g
Copy link
Member

I am going to set all of your PRs related to time-nanos to Draft state, so they don't get merged accidentally before the discussion.

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.

3 participants