DavyJonesLocker / postgres_ext-serializers

MIT License
324 stars 30 forks source link

Rails 4.2.0.beta2 compatibility failed #14

Closed zhaohanweng closed 8 years ago

zhaohanweng commented 10 years ago

Using the Rails 4.2.0.beta2, and this gem, anything like: Post.where(id: [1,2,3]) will fail. Everything is fine when using Rails 4.1.6. Maybe this is a rails error. Here's the error:

Loading development environment (Rails 4.2.0.beta2)
>> Post.where(id: [1,2])
(Object doesn't support #inspect)
=>
>> Post.where(id: [1,2]).take
NoMethodError: undefined method `relation' for #<ActiveRecord::ConnectionAdapters::AbstractAdapter::SQLString:0x007fea84b27bb0>
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/postgres_ext-2.3.0/lib/postgres_ext/arel/visitors/to_sql.rb:7:in `visit_Array'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/reduce.rb:13:in `visit'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:625:in `visit_Arel_Nodes_In'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/reduce.rb:13:in `visit'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:783:in `block in inject_join'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:781:in `each'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:781:in `each_with_index'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:781:in `each'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:781:in `inject'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:781:in `inject_join'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:641:in `visit_Arel_Nodes_And'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/reduce.rb:13:in `visit'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:258:in `block in visit_Arel_Nodes_SelectCore'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:257:in `each'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:257:in `each_with_index'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/arel-6.0.0.beta1/lib/arel/visitors/to_sql.rb:257:in `visit_Arel_Nodes_SelectCore'
... 17 levels...
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/railties-4.2.0.beta2/lib/rails/commands/console.rb:9:in `start'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/railties-4.2.0.beta2/lib/rails/commands/commands_tasks.rb:68:in `console'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/railties-4.2.0.beta2/lib/rails/commands/commands_tasks.rb:39:in `run_command!'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/railties-4.2.0.beta2/lib/rails/commands.rb:17:in `<top (required)>'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/activesupport-4.2.0.beta2/lib/active_support/dependencies.rb:248:in `require'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/activesupport-4.2.0.beta2/lib/active_support/dependencies.rb:248:in `block in require'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/activesupport-4.2.0.beta2/lib/active_support/dependencies.rb:233:in `load_dependency'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/activesupport-4.2.0.beta2/lib/active_support/dependencies.rb:248:in `require'
    from /Users/zweng/workspace/testapi/bin/rails:8:in `<top (required)>'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/activesupport-4.2.0.beta2/lib/active_support/dependencies.rb:242:in `load'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/activesupport-4.2.0.beta2/lib/active_support/dependencies.rb:242:in `block in load'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/activesupport-4.2.0.beta2/lib/active_support/dependencies.rb:233:in `load_dependency'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/gems/2.1.0/gems/activesupport-4.2.0.beta2/lib/active_support/dependencies.rb:242:in `load'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/2.1.0/rubygems/core_ext/kernel_require.rb:55:in `require'
    from /usr/local/var/rbenv/versions/2.1.3/lib/ruby/2.1.0/rubygems/core_ext/kernel_require.rb:55:in `require'
    from -e:1:in `<main>'>>
danmcclain commented 9 years ago

There was a release of a beta version of postgres_ext that is Rails 4.2 compatible, if you include specify 2.4.0.beta.1 as the postgres_ext version, it should work

pkrefta commented 9 years ago

I can confirm that beta rellease fixes this problem. Thank you :+1:

juggy commented 9 years ago

Got something related to that:

09:32:46 web.1  | ArgumentError - wrong number of arguments (1 for 2):
09:32:46 web.1  |   arel (6.0.0.beta2) lib/arel/visitors/reduce.rb:6:in `accept'
09:32:46 web.1  |   postgres_ext-serializers (0.0.3) lib/postgres_ext/serializers/active_model/array_serializer.rb:163:in `_arel_to_json_array_arel'
09:32:46 web.1  |   postgres_ext-serializers (0.0.3) lib/postgres_ext/serializers/active_model/array_serializer.rb:112:in `_include_relation_in_root'
09:32:46 web.1  |   postgres_ext-serializers (0.0.3) lib/postgres_ext/serializers/active_model/array_serializer.rb:27:in `_postgres_serializable_array'
09:32:46 web.1  |   postgres_ext-serializers (0.0.3) lib/postgres_ext/serializers/active_model/array_serializer.rb:10:in `to_json'
09:32:46 web.1  |    () Users/juggy/.rvm/gems/ruby-2.1.2@vroom/bundler/gems/active_model_serializers-b6b01d0b7396/lib/active_model/array_serializer.rb:63:in `to_json'

Forcing postgres_ext 2.4.0.beta.1 did not solve this error.

zhaohanweng commented 9 years ago

The postgres_ext 2.4.0.beta fixed this problem. Everything seems fine. :+1: Thank You.

edit: juggy is right. I have the same error as well with arel 6.0.0.

kenips commented 9 years ago

@juggy it seems like accept takes arel, collector in 4.2 whereas it takes only arel in 4.1. Perhaps we need to split array_serializer by Rails version just like postgres_ext @danmcclain ?

danmcclain commented 9 years ago

There is a possibility that postgres_ext-serializers may be completely re-written. This is very an experiment towards offloading serialization on postgresql. I have some other approaches I want to experience with as well, as this may be the wrong approach. I have a feeling that a change in arel between 5 and 6 is the culprit

kenips commented 9 years ago

Yes I meant Arel :). Explicitly this: https://github.com/rails/arel/commit/05b5bb12270b32e094c1c879273e0978dabe5b3b

kenips commented 9 years ago

So @danmcclain what's the recommendation here? "rewrite" is a big word :)

danmcclain commented 9 years ago

If there is a patch for this, I will be willing to accept a PR, I have been really busy lately, and been neglecting this library as a result, unfortunately. I need to play with some other ideas as well, to see how they turn out. I doubt that the code that exists here will continue on in this vein. Even a slight rewrite would drastically change the querying happening here (At minimum, I think a series of smaller queries with the results combined rails side would generate a better result than this single, colossal query). Needless to say, the library is far from production ready :wink:

shivanibhanwal commented 9 years ago

hi @danmcclain @kenips I am facing same issue, I have migrated my app to rails 4.2 and using AMS 0.8.3 and postgres_ext updated to 2.4.0.beta As you have mentioned above about arel change. I agree because of that we are getting error

<ArgumentError: wrong number of arguments (1 for 2)>

However I am wondering what is the solution now? As I see @danmcclain has mentioned complete rewrite so does that mean we cannot upgrade to rails 4.2 if we are using postgres_ext_serializer?

mcm-ham commented 9 years ago

There's a fix here will create a PR once I get some unit tests for this added: https://github.com/crossroads/postgres_ext-serializers/commit/0121fad6c14cf80b6ecaa9fc64b3d8e136b08113

shivanibhanwal commented 9 years ago

@mcm-ham , @danmcclain I still see some issues with it. As I am getting error

ActiveRecord::StatementInvalid (PG::ProtocolViolation: ERROR:  
bind message supplies 0 parameters, but prepared statement "" requires 1

I was looking into the rails discussion for the similar error - https://github.com/rails/rails/issues/15920 Hope that can explain something about it. However at the moment I am unsure whether postgres-ext_serializer is causing issue or rails .

I noticed one of the argument in the sql is not replaced with the value and that causes this issue. it keeps $1 as is instead it should replace it with the actual value

mcm-ham commented 9 years ago

I've updated the commit to implement a quick approach to get it working but probably needs more attention to do it properly.

felixbuenemann commented 8 years ago

Current master supports rails 4.2 as of PR #38.