postgresql_cursor icon indicating copy to clipboard operation
postgresql_cursor copied to clipboard

Different column types and enum issues

Open antulik opened this issue 4 years ago • 5 comments

each_instance incorrectly sets the column types, for example

class User < ActiveRecord::Base
  enum state: {
    active: 1,
  }
end

pp User.first
# #<User:0x00007f884e1bee40
#  id: 1,
#  created_at: Wed, 17 Apr 2019 14:33:34 AEST +10:00
#  state: "active">

pp User.each_instance.first
# #<User:0x00007f884e1bee40
#  id: 1,
#  created_at: 2019-04-16 22:24:59 UTC
#  state: 1>

Expected. in the example above objects from both queries should be identical

rails -v => Rails 6.0.3.4 ruby -v => ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-darwin19]

antulik avatar Dec 22 '20 06:12 antulik

A workaround

User.each_row do |attributes|
  user = User.instantiate attributes
  # do stuff
end

Note: it only works for simple queries where returned columns map directly to model columns

antulik avatar Dec 22 '20 06:12 antulik

Looking at rails code, rails defaults column types instead of relying on postgres.

https://github.com/rails/rails/blob/23019c3969906b032235a644eb73768809e289a6/activerecord/lib/active_record/querying.rb#L50-L52

while postgresql_cursor gem forces all column types from postgres

https://github.com/afair/postgresql_cursor/blob/6123a23d5c338ae27b28c9292ec7d461db22d5f5/lib/postgresql_cursor/cursor.rb#L229-L249

antulik avatar Dec 22 '20 07:12 antulik

We had the same issue with an enum column and I suspect this would do to this as well for our columns using store_model.

Our workaround was to use the cursor instance directly to override the klass param passed to each_instance:

class ModelInstantiator
  def initialize(model)
    @model = model
  end

  def instantiate(row, column_types)
    column_types = column_types.reject { |k, _| @model.attribute_types.key?(k) }
    @model.instantiate(row, column_types)
  end
end

SomeModel.all.each_row(**some_kwargs).each_instance(ModelInstantiator.new(SomeModel)) do |record|
  # .... do something with record
end

JeanSebTr avatar Nov 11 '22 22:11 JeanSebTr

I have an issue with DateTime which is solved by running eg. created_at.in_time_zone.to_fs(:dmy), else it is printing UTC.

activerecord (= 7.0.5.1)

keymastervn avatar Nov 22 '23 13:11 keymastervn

Update: I endup following to what Rails has been doing with column_types

https://github.com/rails/rails/blob/92d204e7ac0275ec6b566559006fe64fa4319259/activerecord/lib/active_record/querying.rb#L60C14-L62

by rejecting if the attribute_types has the mapping keys column_types.reject { |k, _| klass.attribute_types.key?(k) }, the column_types then becomes an empty hash.

    def each_instance(klass = nil, &block)
      klass ||= @type
      each_tuple do |row|
        if ::ActiveRecord::VERSION::MAJOR < 4
          model = klass.send(:instantiate, row)
        else
          @column_types ||= column_types.reject { |k, _| klass.attribute_types.key?(k) }

          model = klass.send(:instantiate, row, @column_types)
        end
        block.call(model)
      end
    end

    def each_instance_batch(klass = nil, &block)
      klass ||= @type
      each_batch do |batch|
        models = batch.map do |row|
          if ::ActiveRecord::VERSION::MAJOR < 4
            klass.send(:instantiate, row)
          else
            @column_types ||= column_types.reject { |k, _| klass.attribute_types.key?(k) }

            klass.send(:instantiate, row, @column_types)
          end
        end
        block.call(models)
      end
    end

and it works

keymastervn avatar Nov 23 '23 09:11 keymastervn