Skip to content

feat(time): Replace time.Time with time.Duration for Go type#1097

Merged
ernado merged 2 commits intomainfrom
kavirajk/fix-time-time64-types
Oct 4, 2025
Merged

feat(time): Replace time.Time with time.Duration for Go type#1097
ernado merged 2 commits intomainfrom
kavirajk/fix-time-time64-types

Conversation

@kavirajk
Copy link
Copy Markdown
Contributor

@kavirajk kavirajk commented Oct 3, 2025

Summary

Currently time and time64 types are treated as Go's time.Time for inserts and scan.

This causes lots of complexity and bad UX for users.

  1. We have to deal with Day, Month and Year eventhough that is not part of original CH time and time64 types
  2. Handling timezone. Which is also not part of CH types.

Having coming up with "sane" zero values is super cumbersome. Plus it's going to be bad UX for developers if want to do some arithmetic on those types.

This PR converts that to more sane time.Duration type. This type fits naturally to CH time and time64 types.

Also see this comment

Checklist

Delete items not relevant to your PR:

@kavirajk kavirajk changed the title feat(time/time64): Replace time.Time with time.Duration for Go type [time/time64]: Replace time.Time with time.Duration for Go type Oct 3, 2025
@kavirajk kavirajk changed the title [time/time64]: Replace time.Time with time.Duration for Go type feat(time): Replace time.Time with time.Duration for Go type Oct 3, 2025
@kavirajk kavirajk marked this pull request as ready for review October 3, 2025 22:18
@kavirajk kavirajk requested a review from SpencerTorres October 3, 2025 22:23
@kavirajk kavirajk force-pushed the kavirajk/fix-time-time64-types branch from f6cfd63 to eafa8f7 Compare October 3, 2025 22:35
@ernado ernado merged commit 920e759 into main Oct 4, 2025
19 checks passed
@ernado ernado deleted the kavirajk/fix-time-time64-types branch October 4, 2025 04:33
@kavirajk
Copy link
Copy Markdown
Contributor Author

Ah. @SpencerTorres had some opinions before merge.

@SpencerTorres feel free to still review and share feedback in same PR, I will address it in the follow up PR

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants