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

[Proposal] Changed valid_in to exclude valid_from = to and valid_to = from #95

Merged
merged 2 commits into from
Jul 15, 2022

Conversation

osyo-manga
Copy link
Collaborator

@osyo-manga osyo-manga commented Jul 6, 2022

Summary

valid_in include valid_from = to and valid_to = from.

Code:  valid_in(from: 02/01, to: 05/01)
Query: from <= valid_to AND valid_from <= to

01/01         02/01         03/01         04/01         05/01         06/01
  |*************|*************|*************|*************|*************|
               ↑                                        ↑
              from(02/01)                                to(05/01)

However, in real-world use cases, you may not want to include valid_from = to and valid_to = from.
e.g. https://github.com/kufu/activerecord-bitemporal/pull/93/files#diff-77aac43b146505859dd74b1fd3e151c0e94174f5abc85475fe1dfba10a5c64bfR513

Proposal

Changed valid_in to exclude valid_from = to and valid_to = from.

Code:  valid_in(from: 02/01, to: 05/01)
Query: from < valid_to AND valid_from < to

01/01         02/01         03/01         04/01         05/01         06/01
  |-------------|*************|*************|*************|-------------|
               ↑                                        ↑
              from(02/01)                                to(05/01)

Also, Range can be passed as an argument.
It behaves differently for (from...to) and (from..to).

Code:  valid_in(02/01...05/01)
Query: from < valid_to AND valid_from < to

01/01         02/01         03/01         04/01         05/01         06/01
  |-------------|*************|*************|*************|-------------|
               ↑                                        ↑
              from(02/01)                                to(05/01)
Code:  valid_in(02/01..05/01)
Query: from < valid_to AND valid_from <= to

01/01         02/01         03/01         04/01         05/01         06/01
  |-------------|*************|*************|*************|*************|
               ↑                                        ↑
              from(02/01)                                to(05/01)

@auto-assign auto-assign bot requested review from kodera123 and kouryou July 6, 2022 10:01
@osyo-manga osyo-manga requested review from wakasa51 and tmtm and removed request for kodera123 and kouryou July 6, 2022 10:16
Copy link

@tmtm tmtm left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@Dooor Dooor requested review from Dooor and removed request for wakasa51 July 15, 2022 06:16
Copy link
Contributor

@Dooor Dooor left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

@osyo-manga
Copy link
Collaborator Author

Thanks for review :)

@osyo-manga osyo-manga merged commit bc1c566 into kufu:master Jul 15, 2022
@osyo-manga osyo-manga deleted the improve-valid_in branch July 15, 2022 06:22
@mkmn mkmn mentioned this pull request Sep 16, 2022
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.

None yet

3 participants