Add support for Postgres Range Types#1534
Add support for Postgres Range Types#1534sirwolfgang wants to merge 2 commits intopaper-trail-gem:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for PostgreSQL range column types (daterange, numrange, tsrange, tstzrange, int4range, int8range) to PaperTrail's versioning system. Previously, these types were not properly serialized/deserialized when tracking changes to model attributes containing range values.
Key changes:
- Introduced
PostgresRangeSerializerto handle serialization/deserialization of PostgreSQL range types - Modified attribute serializers to detect and process range columns even when using JSON storage
- Refactored variable naming from
klass/item_classtomodel_classfor consistency
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/paper_trail/type_serializers/postgres_range_serializer.rb | New serializer class that handles PostgreSQL range types with deserialize logic for Ruby-formatted range strings |
| lib/paper_trail/attribute_serializers/attribute_serializer_factory.rb | Added range type detection and factory method to instantiate PostgresRangeSerializer |
| lib/paper_trail/attribute_serializers/object_attribute.rb | Added logic to serialize range columns even in JSON mode; renamed variables to model_class |
| lib/paper_trail/attribute_serializers/object_changes_attribute.rb | Added logic to serialize range columns even in JSON mode; renamed variables to model_class |
| lib/paper_trail/attribute_serializers/cast_attribute_serializer.rb | Renamed variables from klass to model_class for consistency |
| spec/paper_trail/type_serializers/postgres_range_serializer_spec.rb | Comprehensive tests for deserializing various PostgreSQL range types |
| spec/paper_trail/attribute_serializers/object_attribute_spec.rb | Integration tests for range serialization/deserialization; updated test descriptions |
| CHANGELOG.md | Added entry documenting the fix for range type support |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ### Fixed | ||
|
|
||
| - None | ||
| - Fixed errors when deserializing Range types from Ruby style strings to Postgres |
There was a problem hiding this comment.
The CHANGELOG entry states "Fixed errors when deserializing Range types from Ruby style strings to Postgres" but this is confusing. It should clarify whether it's deserializing FROM Ruby strings or FROM Postgres format strings. Based on the code, it appears to deserialize Ruby-style range strings (e.g., "1..5") to Ruby Range objects. Consider rewording to: "Fixed support for PostgreSQL range column types in object/object_changes serialization"
| - Fixed errors when deserializing Range types from Ruby style strings to Postgres | |
| - Fixed support for PostgreSQL range column types in object/object_changes serialization |
| expect(attrs["post_ids"]).to eq [1, 2, 3] | ||
| end | ||
|
|
||
| it "serializes a postgres range into a ruby array" do |
There was a problem hiding this comment.
The test description says "serializes a postgres range into a ruby array" but it should say "serializes a postgres range into a ruby range" since ranges are being serialized as ranges, not arrays.
| it "serializes a postgres range into a ruby array" do | |
| it "serializes a postgres range into a ruby range" do |
| def deserialize_with_ar(string) | ||
| return nil if string.blank? | ||
|
|
||
| delimiter = string[/\.{2,3}/] |
There was a problem hiding this comment.
The regex pattern /\.{2,3}/ will match 2 or more consecutive dots, which means it could match "....", ".....", etc. This should be /\.{2}|\.{3}/ or more simply /(\.\.\.?)/ to match only ".." or "..." exactly.
| delimiter = string[/\.{2,3}/] | |
| delimiter = string[/(\.\.\.?)/] |
|
This PR has been automatically marked as stale due to inactivity. |
mattmenefee
left a comment
There was a problem hiding this comment.
Thanks for working on this, @sirwolfgang! This is a real bug that affects anyone using PostgreSQL range columns with PaperTrail's JSON version storage. The approach mirrors the existing PostgresArraySerializer pattern nicely, and the test coverage across all six range types is solid.
A few observations and suggestions:
General Feedback
The approach is sound — routing range types through a custom serializer that understands Ruby-style string representations (1..5) rather than PostgreSQL bracket notation ([1,5)) is the right call, since PaperTrail's JSON columns store to_s output.
Specific Suggestions
PostgresRangeSerializer#deserialize_with_ar — edge cases
The delimiter = string[/\.{2,3}/] regex works correctly for well-formed Ruby range strings, but consider what happens if the input is somehow in PostgreSQL bracket format (e.g., [2024-01-01,2024-01-31)) — the delimiter match would return nil, and string.split(nil) would raise. A guard clause or fallback to the original Active Record deserializer might make this more robust:
def deserialize_with_ar(string)
return nil if string.blank?
delimiter = string[/\.{2,3}/]
return @active_record_serializer.deserialize(string) unless delimiter
range_start, range_end = string.split(delimiter, 2)
# ...
endattributes_to_serialize / changes_to_serialize — potential double-serialization
When a column is both encrypted and a range type, it would appear in both encrypted_to_serialize and columns_to_serialize. Since the merge is encrypted_to_serialize.merge(columns_to_serialize), the range version wins, which seems correct — but it's worth adding a brief comment noting this is intentional.
Test: description typo
# Line says "ruby array" but should say "ruby range"
it "serializes a postgres range into a ruby array" doCHANGELOG entry
The current wording — "Fixed errors when deserializing Range types from Ruby style strings to Postgres" — is a bit ambiguous about the direction. Consider:
- [Fixed] Support for PostgreSQL range column types in version object serialization
Missing test coverage (nice-to-haves)
nilrange values- Range with only one bound set (e.g.,
1..in Ruby 2.6+, or unbounded on one side in PostgreSQL) - A full integration test that round-trips through
reify(mirroring the reproduction script in #1486)
Overall this is good work and addresses a legitimate gap. Would love to see it merged!
This Add/Fixes support for Postgres Range types: #1486
Check the following boxes:
master(if not - rebase it).code introduces user-observable changes.
and description in grammatically correct, complete sentences.
Related: #1487