Skip to content

Commit

Permalink
Remove custom TSRange parser
Browse files Browse the repository at this point in the history
This commit removes the custom TSRange parser implemented in Chronomodel
in favor of the standard Rails tsrange parser.

Implementation:
- Keep returning `nil` instead of Infinity values to keep the previous
  behaviour and allow Rails 7.0 to work (ref: rails/rails#45099)
- Check for `Time` because `validity` is a column with `tsrange` type,
  so that only `Time`, `nil`, and Infinity are supposed to be there
- Bump major version because this is a potential breaking change

Motivation:
- Resolved dependencies:
  - Ruby 2.6+ now supports open-ended ranges, addressing the limitation in
    https://bugs.ruby-lang.org/issues/6864
  - Rails tsrange stability issues in rails/rails#13793 and
    rails/rails#14010
- Standardized usage:
  - Aligns Chronomodel with Rails conventions, improving compatibility and
    reducing code complexity.
- Leverage official API:
  - Accesses potential future improvements and bug fixes in the standard
    tsrange.

Considerations:
- Applications relying on validity.last might require migration
  strategies due to open-ended ranges.
- Utilize Chronomodel's validity and valid_from / valid_to for accurate
  handling of open-ended ranges.

Benefits:
- Simplified codebase
- Enhanced Rails compatibility
- Future-proofed against potential API changes

This change strengthens Chronomodel's foundation and paves the way for
seamless integration with the wider Ruby and Rails communities.

Close #269

Ref:
- rails/rails#45099
  • Loading branch information
tagliala committed Feb 23, 2024
1 parent 5f05360 commit 0d559ef
Show file tree
Hide file tree
Showing 6 changed files with 12 additions and 86 deletions.
1 change: 0 additions & 1 deletion .rubocop_todo.yml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 0 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,6 @@ Ensure to run the full test suite before pushing.
of an object at a specific point in time within the application could
lead to inaccuracies.

* Rails 4+ support requires disabling tsrange parsing support, as it
[is broken][r4-tsrange-broken] and [incomplete][r4-tsrange-incomplete]
as of now, mainly due to a [design clash with ruby][pg-tsrange-and-ruby].

* The triggers and temporal indexes cannot be saved in schema.rb. The AR
schema dumper is quite basic, and it isn't (currently) extensible.
As we're using many database-specific features, Chronomodel forces the
Expand Down Expand Up @@ -412,17 +408,13 @@ This software is Made in Italy :it: :smile:.
[pg-exclusion-constraints]: https://www.postgresql.org/docs/9.4/sql-createtable.html#SQL-CREATETABLE-EXCLUDE
[pg-btree-gist]: https://www.postgresql.org/docs/9.4/btree-gist.html
[pg-comment]: https://www.postgresql.org/docs/9.4/sql-comment.html
[pg-tsrange-and-ruby]: https://bugs.ruby-lang.org/issues/6864
[pg-ctes]: https://www.postgresql.org/docs/9.4/queries-with.html
[pg-cte-optimization-fence]: https://www.postgresql.org/message-id/[email protected]
[pg-cte-opt-out-fence]: https://www.postgresql.org/message-id/CAHyXU0zpM5+Dsb_pKxDmm-ZoWUAt=SkHHaiK_DBqcmtxTas6Nw@mail.gmail.com
[pg-json-type]: https://www.postgresql.org/docs/9.4/datatype-json.html
[pg-json-func]: https://www.postgresql.org/docs/9.4/functions-json.html
[pg-json-opclass]: https://github.com/ifad/chronomodel/blob/master/sql/json_ops.sql

[r4-tsrange-broken]: https://github.com/rails/rails/pull/13793#issuecomment-34608093
[r4-tsrange-incomplete]: https://github.com/rails/rails/issues/14010

[cm-readme-sql]: https://github.com/ifad/chronomodel/blob/master/README.sql
[cm-timemachine]: https://github.com/ifad/chronomodel/blob/master/lib/chrono_model/time_machine.rb
[cm-cte-impl]: https://github.com/ifad/chronomodel/commit/18f4c4b
Expand Down
2 changes: 0 additions & 2 deletions lib/chrono_model/adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@

require 'chrono_model/adapter/ddl'
require 'chrono_model/adapter/indexes'
require 'chrono_model/adapter/tsrange'
require 'chrono_model/adapter/upgrade'

module ChronoModel
Expand All @@ -19,7 +18,6 @@ class Adapter < ActiveRecord::ConnectionAdapters::PostgreSQLAdapter
include ChronoModel::Adapter::Migrations
include ChronoModel::Adapter::DDL
include ChronoModel::Adapter::Indexes
include ChronoModel::Adapter::TSRange
include ChronoModel::Adapter::Upgrade

# The schema holding current data
Expand Down
72 changes: 0 additions & 72 deletions lib/chrono_model/adapter/tsrange.rb

This file was deleted.

13 changes: 11 additions & 2 deletions lib/chrono_model/time_machine/history_model.rb
Original file line number Diff line number Diff line change
Expand Up @@ -209,12 +209,21 @@ def record # :nodoc:
current_version
end

# Return `nil` instead of -Infinity/Infinity to preserve current
# Chronomodel behaviour and avoid failures with Rails 7.0 and
# unbounded time ranges
#
# Check if `begin` and `end` are `Time` because validity is a `tsrange`
# column, so it is either `Time`, `nil`, and in some cases Infinity.
#
# Ref: rails/rails#45099
# TODO: consider removing when Rails 7.0 support will be dropped
def valid_from
validity.first
validity.begin if validity.begin.is_a?(Time)
end

def valid_to
validity.last
validity.end if validity.end.is_a?(Time)
end
alias as_of_time valid_to

Expand Down
2 changes: 1 addition & 1 deletion lib/chrono_model/version.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# frozen_string_literal: true

module ChronoModel
VERSION = '3.0.1'
VERSION = '4.0.0'
end

0 comments on commit 0d559ef

Please sign in to comment.