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.

Sign up for GitHub

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

Jump to bottom

Use exact date spans instead of multi-nine approximations for all_day, all_week, etc. methods #51575

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Use exact date spans instead of multi-nine approximations for all_day, all_week, etc. methods #51575

wants to merge 1 commit into from
+57 −22

Conversation

jjb
Copy link
Contributor

@jjb jjb commented Apr 15, 2024 •
edited

Motivation / Background

  • >= AND <= is bad
  • >= AND < is good

https://wiki.postgresql.org/wiki/Don't_Do_This#Don.27t_use_BETWEEN_.28especially_with_timestamps.29

Why is it okay to change this behavior?

I doubt anyone actually wants the possibility of being infinite points of precision away from tomorrow
— @natematykiewicz

ToDo

  • tests for new methods
  • DRY new methods?

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Unrelated changes should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@jjb jjb changed the title exact date spans Use exact date spans except for multi-nine approximations for all_day, all_week, etc. methods Apr 15, 2024
@jjb jjb changed the title Use exact date spans except for multi-nine approximations for all_day, all_week, etc. methods Use exact date spans instead of multi-nine approximations for all_day, all_week, etc. methods Apr 16, 2024
sambostock
sambostock reviewed May 1, 2024
View reviewed changes
Copy link
Contributor

@sambostock sambostock left a comment

Choose a reason for hiding this comment

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

Rather than end_of_X_threshold, based on the implementations, should the methods be called beginning_of_next_X or something similar?

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

@sambostock sambostock sambostock left review comments

Assignees
No one assigned
Labels
activesupport
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@jjb @sambostock