Would not this only fix the problem with the postgresql timestamp? I think the problem happens with any type that uses TimeZoneConverter, which also includes DateTime

Got it, @rafaelfranca

I'll push a new fix for reviews quite soon

@ruyrocha Hey, thanks for the PR! Just got around to taking a look. Perhaps worth noting that this introduces kind of the opposite problem described by #52698.

Before:

subtype = ActiveRecord::Type::DateTime.new
value = ActiveRecord::AttributeMethods::TimeZoneConversion::TimeZoneConverter.new(subtype)
value == "something that doesn't respond to precision/scale/limit/type"
#=> false

After:

subtype = ActiveRecord::Type::DateTime.new
value = ActiveRecord::AttributeMethods::TimeZoneConversion::TimeZoneConverter.new(subtype)
value == "something that doesn't respond to precision/scale/limit/type"
#=> NoMethodError: undefined method `precision' for an instance of String

I guess there might be arguments about duck typing, but for a method like #==, I'd say it's probably a good idea to be resilient to comparing two completely unrelated values. 🤷 The class check of the original method would catch such problems, but in a way that's too strict for the DelegateClass usage, which results in false negatives. Maybe an other.is_a?(ActiveModel::Type::Value) at the front of this definition would suffice?

Thanks. Does this fix the issue you were having and the new one?

diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
index 27e83ec6dd..26e974d83a 100644
--- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
+++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb
@@ -33,10 +33,7 @@ def cast(value)
         end
 
         def ==(other)
-          precision == other.precision &&
-            scale == other.scale &&
-            limit == other.limit &&
-            type == other.type
+          other.is_a?(self.class) && __getobj__ == other.__getobj__
         end
 
         private
diff --git a/activerecord/test/cases/attribute_methods/time_zone_converter_test.rb b/activerecord/test/cases/attribute_methods/time_zone_converter_test.rb
index 5deb026f15..dac23f1ab2 100644
--- a/activerecord/test/cases/attribute_methods/time_zone_converter_test.rb
+++ b/activerecord/test/cases/attribute_methods/time_zone_converter_test.rb
@@ -13,6 +13,7 @@ def test_comparison_with_date_time_type
           value_from_cache = Marshal.load(Marshal.dump(value))
 
           assert_equal value, value_from_cache
+          assert_not_equal value, "foo"
         end
       end
     end

It looks good, @rafaelfranca, and I think it'll fix the issues @ajvondrak was facing. I'd only put it first to be consistent with other #== methods, e.g:

        def ==(other)
          other.is_a?(self.class) && __getobj__ == other.__getobj__ &&
            precision == other.precision &&
            scale == other.scale &&
            limit == other.limit &&
            type == other.type
        end

I'm happy to push it if you don't have any objections

Hmm, I suppose other.is_a?(self.class) would technically change another piece of behavior from the old version in the case of comparing a timezone-wrapped datetime to just a raw datetime.

Before:

unwrapped = ActiveRecord::Type::DateTime.new
wrapped = ActiveRecord::AttributeMethods::TimeZoneConversion::TimeZoneConverter.new(unwrapped)

# since wrapped.== delegates to the underlying unwrapped.==, this is equivalent to unwrapped == unwrapped:
wrapped == unwrapped
#=> true

# but unwrapped.== doesn't know to unwrap the wrapped value:
unwrapped == wrapped
#=> false

After:

unwrapped = ActiveRecord::Type::DateTime.new
wrapped = ActiveRecord::AttributeMethods::TimeZoneConversion::TimeZoneConverter.new(unwrapped)

# now, unwrapped.is_a?(TimeZoneConverter) fails:
wrapped == unwrapped
#=> false

# this is unchanged:
unwrapped == wrapped
#=> false

I think the After behavior actually makes more sense, since it keeps == symmetric. So yeah, I can't think of any other gotchas offhand with the is_a? + __getobj__ == __getobj__ check. 👍

@ruyrocha FWIW, I don't think that the precision/scale/limit/type comparisons are necessary with the proposed definition given that we'd be delegating to __getobj__ == other.__getobj__, which will already house its own logic for comparing those attributes.

@ajvondrak @rafaelfranca #52726 is ready, but I don't know how to proceed:

• ActiveRecord::AttributeMethods::TimeZoneConversion::TimeZoneConverter equality is buggy #52698 needs to be open again?
• a separate issue?