afair / postgresql_cursor

ActiveRecord PostgreSQL Adapter extension for using a cursor to return a large result set
MIT License
588 stars 48 forks source link

Different column types and enum issues #57

Open antulik opened 3 years ago

antulik commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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

JeanSebTr commented 1 year ago

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
keymastervn commented 9 months ago

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 commented 9 months ago

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