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

Unknown type Jsonb when using Attributes API. #267

Closed
jhirn opened this issue Nov 6, 2017 · 12 comments · Fixed by #334
Closed

Unknown type Jsonb when using Attributes API. #267

jhirn opened this issue Nov 6, 2017 · 12 comments · Fixed by #334

Comments

@jhirn
Copy link

jhirn commented Nov 6, 2017

Hi.

I'm having an issue when declaring a jsonb attribute using the attributes api and the postgis adapter. This works fine when using the postgresql adapter.

If I have a model defined like this

class AttributeBug < ApplicationRecord
  attribute :foobar, :jsonb
end

I get the following error in rails console

irb(main):003:0> AttributeBug.new
ArgumentError: Unknown type :jsonb
	from (irb):3
irb(main):004:0> AttributeBug.new
=> #<AttributeBug id: nil, created_at: nil, updated_at: nil>

Note the 2nd call doesn't fail, although the model does not have the foobar attribute defined on it.

I've created a sample app here to recreate the problem. I tried to dig through this and the pg gem to see the difference in how the ActiveRecord::Types are registered, but couldn't figure it out. I'll continue to search on my own but wanted to open an issue on it for now.

https://github.com/jhirn/postgis_jsonb_attribute_bug/blob/master/README.md

@jhirn
Copy link
Author

jhirn commented Nov 6, 2017

If I load this class from AR master and register it with ActiveRecord::Type.register(:jsonb, ActiveRecord::Type::Json) everything seems to work.

@teeparham
Copy link
Member

teeparham commented Nov 6, 2017

Thanks for reporting this issue.

This adapter is a subclass of the Postgres adapter: class PostGISAdapter < PostgreSQLAdapter. The types are registered by the postgresql adapter here:
https://github.com/rails/rails/blob/master/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb

I'm not sure what the cause of the problem is, but maybe that will help.

@jhirn
Copy link
Author

jhirn commented Nov 6, 2017

Hey thanks for the tip (and the gem). I'll try more debugging this afternoon.

@jhirn
Copy link
Author

jhirn commented Nov 7, 2017

Hey @teeparham.

I think I've found the issue, not sure how you'd like to go about resolving it in your adapter. The postgresql adapter registers these native types specific to the postgres adapter as such:

ActiveRecord::Type.register(:jsonb, OID::Jsonb, adapter: :posgresql)

The source for that is here

I've found by registering these with the adapter set to postgis, the problem goes away.

module ActiveRecord
  module ConnectionAdapters
    ActiveRecord::Type.register(:jsonb, PostgreSQL::OID::Jsonb, adapter: :postgis)
    #and so on.....
  end
end

I guess you could just duplicate the registrations in the PostGIS connection adapter. It's unlikely they'll suffer from a ton of churn. It would be a lot nicer maintenance wise if you could read metadata from the PostgreSQL registry and repeat them for PostGIS adapter. Not exactly sure how to do that but going to poke around. This is deeper than I've had to go before in AR ;)

If I find a nice solution I'll issue a PR for you

@jhirn
Copy link
Author

jhirn commented Nov 8, 2017

I couldn't find a way to read decent way to read the registry and get the collection of types initialized in PostgreSQLAdapter registered with PostGIS. For now I've monkey patched it in my project. Here's my fork with the commit. Happy to make a PR if it's suitable. Please note for 4.0-stable you have to omit legacy_point type. I believe it's not available in AR 5.0.

jhirn@04a8cf0

@teeparham
Copy link
Member

I still don't have a solution but here's what I found/confirmed:

The types are registered to the "postgresql" adapter (even though we're using "postgis"). New spatial types are not registered. Ideally the upstream type registration in the PostgreSQLAdapater would work for subclassed adapters, & then the PostGIS adapter could add the new spatial types.

# ActiveRecord 5.0
ActiveRecord::Type.registry.send(:registrations).map{|r| [r.send(:adapter), r.send(:name)].join(" : ")}
[
  [ 0] " : big_integer",
  [ 1] " : binary",
  [ 2] " : boolean",
  [ 3] " : date",
  [ 4] " : datetime",
  [ 5] " : decimal",
  [ 6] " : float",
  [ 7] " : integer",
  [ 8] " : string",
  [ 9] " : text",
  [10] " : time",
  [11] "postgresql : ",
  [12] "postgresql : ",
  [13] "postgresql : bit",
  [14] "postgresql : bit_varying",
  [15] "postgresql : binary",
  [16] "postgresql : cidr",
  [17] "postgresql : datetime",
  [18] "postgresql : decimal",
  [19] "postgresql : enum",
  [20] "postgresql : hstore",
  [21] "postgresql : inet",
  [22] "postgresql : json",
  [23] "postgresql : jsonb",
  [24] "postgresql : money",
  [25] "postgresql : point",
  [26] "postgresql : legacy_point",
  [27] "postgresql : uuid",
  [28] "postgresql : vector",
  [29] "postgresql : xml"

It seems possible to alter the type registry when the PostGIS adapter is loaded (change "postgresql" to "postgis"). For example, this hackery works:

ActiveRecord::Type.registry.send(:registrations).last.instance_variable_set("@adapter", "postgis")

@jhirn
Copy link
Author

jhirn commented Dec 12, 2017 via email

@keithdoggett
Copy link
Member

With #334, the adapter will now check for types registered under the :postgis or :postgresql adapters.

@jhirn
Copy link
Author

jhirn commented Mar 4, 2021 via email

@SampsonCrowley
Copy link

@keithdoggett has there been a new release since that was merged? I just ran into this in combination with another gem (that relies on type lookups) not being able to find type uuid (full error: ArgumentError (Unknown type :uuid)) that instantly goes away when switched to :postgresql instead of :postgis

@SampsonCrowley
Copy link

Even just throwing the types registration into an override module fixes it for me though, so the fix is definitely functional! just either waiting on or missed the next release

@keithdoggett
Copy link
Member

@SampsonCrowley this has been released in 7.1.0 and is only in the master branch currently.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants