Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding array enum column in same migration as enum creation fails with "TypeError: can't quote Array" #58

Open
anaulin opened this issue Jul 14, 2022 · 7 comments
Labels
bug Something isn't working

Comments

@anaulin
Copy link

anaulin commented Jul 14, 2022

Describe the bug
The following migration:

def change
  create_enum :product_type, %w[one-off subscription]

  add_column :products, :types, :product_type, array: true, default: []
end

Fails with the error:

TypeError: can't quote Array

However, separating the create_enum into its own migration, running it, and then separately running a migration with add_column succeeds. (Running the two migrations together also fails.)

I did some exploration, and it seems the problem is that Rails doesn't know the type of the :types column when it is trying to serialize the default value for that column. If I rewrite the above migration as:

  create_enum :product_type, %w[one-off subscription]

  add_column :products, :types, :product_type, array: true

  Product.reset_column_information
  change_column_default :products, :types, default: []

This fails with the errors:

unknown OID 697877: failed to recognize type of 'types'. It will be treated as String.
rails aborted!
StandardError: An error has occurred, this and all later migrations canceled:

can't quote Hash

Expected behavior
I would expect this migration to work without having to first fully apply the create_enum line.

Context (please complete the following information):

  • OS: Debian GNU/Linux 11 (bullseye)
  • Rails 6.1.6.1
  • activerecord-postgres_enum (2.0.1)
  • postgres:13.5
@bibendi bibendi added the bug Something isn't working label Jul 14, 2022
@bibendi
Copy link
Owner

bibendi commented Jul 14, 2022

Thank you for the report!

@anaulin
Copy link
Author

anaulin commented Jul 18, 2022

(Added some more details to description, from my own debugging.)

@attilahorvath
Copy link
Contributor

I have encountered this same issue and tracked it down to activerecord. It is trying to load the array type from PostgreSQL before the enum type is loaded, which makes it default to string, breaking the code that uses the array.

I have a small patch that seems to resolve this by making sure the array included element types are always loaded before the arrays themselves. Hope this helps someone out.

Here is the patch for rails 6.1:

# https://github.com/rails/rails/blob/v6.1.0/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L625-L643
raise 'This Monkeypatch is only tested with activerecord 6.1.x' unless Gem.loaded_specs['activerecord'].version.to_s.starts_with?('6.1.')

module ActiveRecord
  module ConnectionAdapters
    class PostgreSQLAdapter
      def load_additional_types(oids = nil)
        initializer = OID::TypeMapInitializer.new(type_map)

        query = <<~SQL
          SELECT t.oid, t.typname, t.typelem, t.typdelim, t.typinput, r.rngsubtype, t.typtype, t.typbasetype
          FROM pg_type as t
          LEFT JOIN pg_range as r ON oid = rngtypid
        SQL

        if oids
          query += "WHERE t.oid IN (%s)" % oids.join(", ")
        else
          query += initializer.query_conditions_for_initial_load
        end

        execute_and_clear(query, "SCHEMA", []) do |records|
          elem_oids = records.map { |row| row['typelem'] }.compact
          load_additional_types(elem_oids) if oids && elem_oids.any?

          initializer.run(records)
        end
      end
    end
  end
end

In rails 7.1 this method was simplified. This should work with that new version:

# https://github.com/rails/rails/blob/v7.1.0/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L844-L851
raise 'This Monkeypatch is only tested with activerecord 7.1.x' unless Gem.loaded_specs['activerecord'].version.to_s.starts_with?('7.1.')

module ActiveRecord
  module ConnectionAdapters
    class PostgreSQLAdapter
      def load_additional_types(oids = nil)
        initializer = OID::TypeMapInitializer.new(type_map)
        load_types_queries(initializer, oids) do |query|
          execute_and_clear(query, "SCHEMA", [], allow_retry: true, materialize_transactions: false) do |records|
            elem_oids = records.map { |row| row['typelem'] }.compact
            load_additional_types(elem_oids) if oids && elem_oids.any?

            initializer.run(records)
          end
        end
      end
    end
  end
end

As this is an issue in activerecord really and could happen without using this gem, I don't think there is a need to work around it in the gem. I'll report it to the activerecord maintainers rather to include a fix.

@princejoseph
Copy link

@attilahorvath Did you end up reporting this to Rails by any chance?

@silent-e
Copy link

silent-e commented Nov 13, 2024

I was able to define an enum array out of the box with Rails 7.1.4.2 in the same migration.

Migration, which works in 7.1 as described in the Rails guide on Postgres. The guide doesn't mention array: true but it worked.

create_enum "location", ["Nationwide", "Virtual", "California"]

create_table "users" do |t|
    t.enum "location", enum_type: "location", array: true, null: false
end

However there's a lot of weirdness once you try saving and reading saved values. I couldn't define an enum then assign to the column with an array.

This definition breaks things when the column is an array.

enum location: {
  nationwide: 'Nationwide',
  virtual: 'Virtual',
  california: 'California',
}

The following line doesn't work because assert_valid_value doesn't recognize array values.

user.location = ['California']

Adding validate: true to the enum definition in the model does let me assign as an array but then validation fails because it validates on the enum key. The fix there is to define the enum in the model with matching keys and values. Saving appears to work then.

But then loading a record with successfully saved array values returns nil for that attribute. Doing nothing other than commenting out the model enum definition and reloading the record does return the values as an array.

So, there appears to be some big disconnect between saving/reading abilities with enum arrays. Looks like quite a bit of work is needed for Rails to support them.

@princejoseph
Copy link

princejoseph commented Nov 13, 2024

I was able to define an enum array out of the box with Rails 7.1.4.2 in the same migration.

Migration, which works in 7.1 as described in the Rails guide on Postgres. The guide doesn't mention array: true but it worked.

create_enum "location", ["Nationwide", "Virtual", "California"]

create_table "users" do |t|
    t.enum "location", enum_type: "location", array: true, null: false
end

However there's a lot of weirdness once you try saving and reading saved values. I couldn't define an enum then assign to the column with an array.

This definition breaks things when the column is an array.

enum location: {
  nationwide: 'Nationwide',
  virtual: 'Virtual',
  california: 'California',
}

The following line doesn't work because assert_valid_value doesn't recognize array values.

user.location = ['California']

Adding validate: true to the enum definition in the model does let me assign as an array but then validation fails because it validates on the enum key. The fix there is to define the enum in the model with matching keys and values. Saving appears to work then.

But then loading a record with successfully saved array values returns nil for that attribute. Doing nothing other than commenting out the model enum definition and reloading the record does return the values as an array.

So, there appears to be some big disconnect between saving/reading abilities with enum arrays. Looks like quite a bit of work is needed for Rails to support them.

@silent-e I dont think enum and associated method in model makes sense for array enums. It is meant for single enum and creates methods like user.nationwide? which does not make sense for an array like %w[Nationwide California] (does this return true or false, as it is not just Nationwide). If they support enum array in the future, they will probably add a new method something like enum_array which gives a different set of validations and methods suited for arrays

tldr; Rails model enum method is based on single enum columns, but Rails support array columns, so if you remove enum method, it will work fine except for model validation

@silent-e
Copy link

Agreed. Was just hoping it'd work. I do like the enum_array method idea though. Thanks. For now I'm probably just not going to define the enum in the model since it works that way. I don't need to generated convenience methods anyways.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

5 participants