alecdotninja / active_record_distinct_on

Support for `DISTINCT ON` statements when querying with ActiveRecord
MIT License
34 stars 11 forks source link

pg 1.0.0 causes a strange issue, possibly interacting with this gem #2

Closed kshahkshah closed 6 years ago

kshahkshah commented 6 years ago

Hello, apologies in advance for the lame description and weak stack trace I'm going to provide.

I upgraded from Rails 5.1.2 to 5.1.5 and pg 0.21 to pg 1.0.0 and all appeared to be fine, however upon deleting a user, and note the User model does not use AR Distinct On nor does it cascade and cause other models to, an error was raised.

ArgumentError: unknown relation value :distinct_on

[PROJECT_ROOT]/vendor/ruby-2.3.4/lib/ruby/2.3.0/monitor.rb:214 :in `mon_synchronize`
212     mon_enter
213     begin
214       yield
215     ensure
216       mon_exit
[PROJECT_ROOT]/app/controllers/admin/users_controller.rb:72 :in `destroy`
[GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/relation/query_methods.rb:1200 :in `block in default_value_for`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/relation/query_methods.rb:1199 :in `fetch`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/relation/query_methods.rb:1199 :in `default_value_for`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/relation/query_methods.rb:923 :in `get_value`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/relation.rb:494 :in `block in delete_all`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/relation.rb:493 :in `select`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/relation.rb:493 :in `delete_all`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/persistence.rb:550 :in `destroy_row`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/counter_cache.rb:191 :in `destroy_row`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/locking/optimistic.rb:117 :in `destroy_row`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/persistence.rb:195 :in `destroy`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/callbacks.rb:321 :in `block in destroy`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/callbacks.rb:131 :in `run_callbacks`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/callbacks.rb:827 :in `_run_destroy_callbacks`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/callbacks.rb:321 :in `destroy`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/transactions.rb:303 :in `block in destroy`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/transactions.rb:384 :in `block in with_transaction_returning_status`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/connection_adapters/abstract/database_statements.rb:235 :in `block in transaction`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/connection_adapters/abstract/transaction.rb:194 :in `block in within_new_transaction`
[PROJECT_ROOT]/vendor/ruby-2.3.4/lib/ruby/2.3.0/monitor.rb:214 :in `mon_synchronize`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/connection_adapters/abstract/transaction.rb:191 :in `within_new_transaction`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/connection_adapters/abstract/database_statements.rb:235 :in `transaction`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/transactions.rb:210 :in `transaction`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/transactions.rb:381 :in `with_transaction_returning_status`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/transactions.rb:303 :in `destroy`
[PROJECT_ROOT]/app/controllers/admin/users_controller.rb:72 :in `destroy`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_controller/metal/basic_implicit_render.rb:4 :in `send_action`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/abstract_controller/base.rb:186 :in `process_action`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_controller/metal/rendering.rb:30 :in `process_action`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/abstract_controller/callbacks.rb:20 :in `block in process_action`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/callbacks.rb:131 :in `run_callbacks`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/abstract_controller/callbacks.rb:19 :in `process_action`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_controller/metal/rescue.rb:20 :in `process_action`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_controller/metal/instrumentation.rb:32 :in `block in process_action`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/notifications.rb:166 :in `block in instrument`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/notifications/instrumenter.rb:21 :in `instrument`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/notifications.rb:166 :in `instrument`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_controller/metal/instrumentation.rb:30 :in `process_action`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_controller/metal/params_wrapper.rb:252 :in `process_action`
 [GEM_ROOT]/gems/activerecord-5.1.5/lib/active_record/railties/controller_runtime.rb:22 :in `process_action`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/abstract_controller/base.rb:124 :in `process`
 [GEM_ROOT]/gems/actionview-5.1.5/lib/action_view/rendering.rb:30 :in `process`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_controller/metal.rb:189 :in `dispatch`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_controller/metal.rb:253 :in `dispatch`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/routing/route_set.rb:49 :in `dispatch`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/routing/route_set.rb:31 :in `serve`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/journey/router.rb:50 :in `block in serve`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/journey/router.rb:33 :in `each`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/journey/router.rb:33 :in `serve`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/routing/route_set.rb:844 :in `call`
 [GEM_ROOT]/gems/warden-1.2.7/lib/warden/manager.rb:36 :in `block in call`
 [GEM_ROOT]/gems/warden-1.2.7/lib/warden/manager.rb:35 :in `catch`
 [GEM_ROOT]/gems/warden-1.2.7/lib/warden/manager.rb:35 :in `call`
 [GEM_ROOT]/gems/rack-2.0.4/lib/rack/etag.rb:25 :in `call`
 [GEM_ROOT]/gems/rack-2.0.4/lib/rack/conditional_get.rb:38 :in `call`
 [GEM_ROOT]/gems/rack-2.0.4/lib/rack/head.rb:12 :in `call`
 [GEM_ROOT]/gems/rack-2.0.4/lib/rack/session/abstract/id.rb:232 :in `context`
 [GEM_ROOT]/gems/rack-2.0.4/lib/rack/session/abstract/id.rb:226 :in `call`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/cookies.rb:613 :in `call`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/callbacks.rb:26 :in `block in call`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/callbacks.rb:97 :in `run_callbacks`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/callbacks.rb:24 :in `call`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/debug_exceptions.rb:59 :in `call`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/show_exceptions.rb:31 :in `call`
 [GEM_ROOT]/gems/railties-5.1.5/lib/rails/rack/logger.rb:36 :in `call_app`
 [GEM_ROOT]/gems/railties-5.1.5/lib/rails/rack/logger.rb:24 :in `block in call`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/tagged_logging.rb:69 :in `block in tagged`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/tagged_logging.rb:26 :in `tagged`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/tagged_logging.rb:69 :in `tagged`
 [GEM_ROOT]/gems/railties-5.1.5/lib/rails/rack/logger.rb:24 :in `call`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/remote_ip.rb:79 :in `call`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/request_id.rb:25 :in `call`
 [GEM_ROOT]/gems/rack-2.0.4/lib/rack/method_override.rb:22 :in `call`
 [GEM_ROOT]/gems/rack-2.0.4/lib/rack/runtime.rb:22 :in `call`
 [GEM_ROOT]/gems/activesupport-5.1.5/lib/active_support/cache/strategy/local_cache_middleware.rb:27 :in `call`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/executor.rb:12 :in `call`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/static.rb:125 :in `call`
 [GEM_ROOT]/gems/rack-2.0.4/lib/rack/sendfile.rb:111 :in `call`
 [GEM_ROOT]/gems/actionpack-5.1.5/lib/action_dispatch/middleware/ssl.rb:66 :in `call`
 [GEM_ROOT]/gems/railties-5.1.5/lib/rails/engine.rb:522 :in `call`
 [GEM_ROOT]/gems/puma-3.11.2/lib/puma/configuration.rb:225 :in `call`
 [GEM_ROOT]/gems/puma-3.11.2/lib/puma/server.rb:624 :in `handle_request`
 [GEM_ROOT]/gems/puma-3.11.2/lib/puma/server.rb:438 :in `process_client`
 [GEM_ROOT]/gems/puma-3.11.2/lib/puma/server.rb:302 :in `block in run`
 [GEM_ROOT]/gems/puma-3.11.2/lib/puma/thread_pool.rb:120 :in `block in spawn_thread`

I solved this of course by reverting. And while I recognize that this gem was not directly in the stack trace, it appears that is may be involved? So at the least I'm documenting here for others. Will also open an issues at rails/rails/

alecdotninja commented 6 years ago

Thanks for the bug report, @whistlerbrk. Glancing at the stack trace, I think that some minor refactoring in ActiveRecord is the cause of this issue.

Can you confirm that running ActiveRecord::QueryMethods::DEFAULT_VALUES[:distinct_on] = ActiveRecord::QueryMethods::FROZEN_EMPTY_ARRAY in the console solves the problem?

If so, I'll release a new version with the necessary modifications.

michaelherold commented 6 years ago

I think this is what caused the problem.

https://github.com/rails/rails/commit/df712340bde5ee306456099e0a8e47762515f9c3

Since they are now memoizing values upon load, this monkey patch doesn't add the lookup to the cache any more (because we've already built up the cache by the time the gem is activated).

I performed the hack you suggested and it fixed the problem, so I think that's a shippable change.

I think you'll want do do a version check against Rails for adding to DEFAULT_VALUES since it doesn't exist prior to 5.1.5.

alassek commented 6 years ago

Or just do an existence check for ActiveRecord::QueryMethods::DEFAULT_VALUES

alecdotninja commented 6 years ago

@whistlerbrk A fix for this issue has been released in 0.1.3. :tada: