activerecord-hackery / squeel

Active Record, improved. Live again :)
http://erniemiller.org/2013/11/17/anyone-interested-in-activerecord-hackery/
MIT License
2.4k stars 213 forks source link

Fixed missing bind values when using #in with scope that joins polymorphic association #380

Open andriusch opened 9 years ago

andriusch commented 9 years ago

This might also fix some other reported errors with bind values (#344 for example)

rhomeister commented 9 years ago

Tested this in my code. It solved a large number of PG::ProtocolViolation errors. Thanks! Would be great if this could be merged and released.

rogierslag commented 9 years ago

This might also fix https://github.com/activerecord-hackery/squeel/issues/369

rogierslag commented 9 years ago

This breaks some queries!

eg

SELECT "users".* 
FROM   "users" 
       INNER JOIN "orders" 
               ON "orders"."user_id" = "users"."id" 
WHERE  "users"."id" = 'Shop' 
       AND ( "users"."id" = 201 
              OR "orders"."id" IN (SELECT "orders"."id" 
                                   FROM   "orders" 
                                   WHERE  ( "orders"."user_id" = 201 
                                             OR ( "orders"."shop_id" IN 
                                                  (SELECT "shops"."id" 
                                                   FROM   "shops" 
                                                  LEFT OUTER JOIN 
                                                  "access_rights" 
                                                               ON 
"access_rights"."accessible_id" 
= 
"shops"."id" 
AND 
"access_rights"."accessible_type" 
= 
'Event' 
                       WHERE 
( "shops"."event_id" IN 
  (SELECT "events"."id" 
   FROM   "events" 
  LEFT OUTER JOIN "access_rights" 
               ON 
  "access_rights"."accessible_id" 
  = 
  "events"."id" 
  AND 
"access_rights"."accessible_type" = 
'Organization' 
       WHERE  ( 
"access_rights"."user_id" = 201 
OR "events"."organization_id" IN 
(SELECT 
"organizations"."id" 
FROM 
"organizations" 
LEFT OUTER JOIN "access_rights" 
 ON 
"access_rights"."accessible_id" = 
"organizations"."id" 
AND 
"access_rights"."accessible_type" = 
201 
 WHERE  ( 
"access_rights"."user_id" 
= 201 
AND 
"access_rights"."accessible_type" = 
'Organization' )) ) 
ORDER  BY events.created_at) 
OR "access_rights"."user_id" = 201 ) 
ORDER  BY shops.created_at) 
AND "orders"."status" = 4 ) ) 
ORDER  BY orders.created_at ASC, 
orders.id ASC) ) 
ORDER  BY created_at 
LIMIT  1 

Note the match between a user_id and the string 'Shop'

andriusch commented 9 years ago

I'd be interested to debug why it breaks that query if you can isolate the problem a bit and give me a sample code that breaks.

rogierslag commented 9 years ago

In this case it a policy_scope(User).find(params[:id]) which in turn requests some other stuff.

It looks like the find or where will always be added at the front (so the parameter binding should be done at the front as well) compared to the joins.

I had a similar problem as you to start with, and made a complete failing ruby test. See https://github.com/inventid/squeel-bug-369-testcase for that

andriusch commented 9 years ago

I checked your project and I can't reproduce your bug. policy_scope(User).find(params[:id]) generates SELECT "users".* FROM "users" WHERE "users"."id" = 1 AND "users"."id" = ? LIMIT 1 [["id", 1]] so I don't know what you're referring to. Also get rid of any third party gems when creating a demo. People can't be expected to learn all third party libraries (like pundit for example) when looking into your bug. Isolate what causes your problem, then I can look into it.

shepmaster commented 9 years ago

(#344 for example)

Unfortunately, this PR does not seem to help with #344

jonatack commented 9 years ago

For what it's worth, I updated Polyamorous, the polymorphic library that Squeel depends on, for Rails 5.0 today and while doing so, found a potential small regression in polymorphic joins that I mistakenly made last April.

Don't hesitate to try Polyamorous master with Squeel (add gem 'polyamorous', github: 'activerecord-hackery/polyamorous' to your Gemfile next to gem 'squeel') and let me know.

Thanks!

andriusch commented 9 years ago

Tried, but can't test it, polyamorous is version 1.2.0 not, but squeel depends on ~1.1.0

allenwq commented 8 years ago

Experiencing the same issue, hope this can be merged soon.