Polymorphism broken with Rails 4.2

[rogierslag]

We have the following scenario. A user can have access to a shop. This is done with an polymorphic AccessRight class (accessible_type = 'Shop'). However, it seems on Rails 4.2 Squeel can no longer figure out what the type should be

The scope looks like this

scope :crewed_by, -> (user) {
    joins  { access_rights.accessible(Shop).outer }
    .where {
      (access_rights.user == user) 
    }
  }

SELECT "custom_field_answers".*
FROM "custom_field_answers"
WHERE "custom_field_answers"."order_id" IN (
  SELECT "orders"."id"
  FROM "orders"
  LEFT JOIN "shops" ON "shops"."id" = "orders"."shop_id"
  WHERE (
    "orders"."user_id" = 1
    OR (
     "shops"."id" IN (
      SELECT "shops"."id"
      FROM "shops"
      LEFT JOIN "access_rights" ON "access_rights"."accessible_id" = "shops"."id"
       AND "access_rights"."accessible_type" =
      LEFT JOIN "shops" "accessibles_access_rights" ON "accessibles_access_rights"."id" = "access_rights"."accessible_id"
       AND "access_rights"."accessible_type" = 'Shop'
      WHERE "access_rights"."user_id" = 1
      )
     AND "orders"."status" = 4
     )
    )
  )

As you can see, one time the accessible_type isnt set at all, resulting in invalid SQL

In Rails 4.1, we simply did

cope :crewed_by, -> (user) {
    joins  { access_rights.outer }
    .where {
      (access_rights.user == user) 
    }
  }

@joostverdoorn for info

[jonatack]

Hello @rogierslag,
Does the issue occur in Rails 4.2.0 or 4.2.1?
Do you have a test that passes in 4.1 and fails in 4.2.0 or 4.2.1? A stack trace?
Thanks.

[rogierslag]

It happens both in 4.2.1 as well as 4.2 itself

The error is in fact

     Failure/Error: get :show, id: id, format: :json
     ActiveRecord::RecordNotFound:
       Couldn't find CustomFieldAnswer with 'id'=1 [WHERE "custom_field_answers"."order_id" IN (WHERE ("orders"."user_id" = 1 OR ("shops"."id" IN (WHERE "access_rights"."user_id" = 1) AND "orders"."status" = 4)))]

which is weird, since the entire SQL query is invalid.

I do have tests, but no minimum example, our tests which fails on this kinda travels through the entire application

[jonatack]

Apologies, I deleted my last reply. For some reason, I thought this was the Ransack issue tracker instead of the Squeel one. Sorry, it's late here ;)

[rogierslag]

Still, I'll try to see if I can create a minimum tests which succeeds in 4.1.x and fails in 4.2.x ;)

[jonatack]

That would be great. This could be an issue in Polyamorous too. cc @bigxiang

[marcusg]

Running into the same issue :( It is working with Rails 4.1.10, but fails with 4.2.1

[rogierslag]

Ill try to create a testcase tomorrow (Europe/Amsterdam region). Cant assign it to myself so please ping me if I havent reported back by the end of the week!

[marcusg]

@rogierslag ping :)

[rogierslag]

I'm currently compiling a test case. It is taking quite some time though since I need to take a copy of the api, and strip it down extensively before we can allow it to be opensourced

oops: Forgot to press comment. Anyway, I finished and a test case is located over here https://github.com/inventid/squeel-bug-369-testcase

[rogierslag]

Some explanaiton: by company policy the project has a Vagrantfile so you can easily use Vagrant with Virtualbox to get a fresh VM, just run vagrant up in the repo root. The machine will auto install and in the end give you the remaining instructions on how to trigger the bug

The box ships with Rails 4.2.1, in you downgrade Rails in the Vagrantfile to 4.1.7 (and do bundle update) you will see the bug no longer triggers when running time rspec

[rogierslag]

And finally, Rails versions 4.1.10 does not have the problem. That might further reduce the number of things you will have to check

[rogierslag]

And progress on this one?

[jonatack]

@rogierslag If so, you would have seen it here. Someone who is experiencing this issue will be most likely to work on it. For others, it's a free time thing.

[rogierslag]

I've been debugging for some time now, and it seems the problem boils down to the following. The scopes by pundit are actually handled nice, and the value is in the set. When callign find(1) on the set though, it isn't anymore.

This is caused since the query is changed in the form WHERE id = ? AND (previous conditions). However, the binds are still done in the old fashion (at the end). It looks like changing some ordering in 4.2/relation_extensions.rb fixes the issue, depending on the depth of the joins and ins in the query.

The specific method is expand_attrs_from_hash and it sometimes works by switching the assignments to self.bind_values

I'll try to recreate the specific test case without pundit and other libraries, and provide seeds and tests as well

[rogierslag]

I have recreated the test case, this time is should help you much better. Just start the vagrant machine, follow the instructions after the machine is up (effectively installing the gems and running rspec). I have included 5 testcases, 3 of which are correct, and two which are failing. Finally I've also added a Travis hook which tests on multiple Rails versions.

The code is still located at https://github.com/inventid/squeel-bug-369-testcase
The travis CI build status is located at https://travis-ci.org/inventid/squeel-bug-369-testcase/

@joostverdoorn for info

[rogierslag]

Rails 4.2.4 did not fix the bug. Test result are located here https://travis-ci.org/inventid/squeel-bug-369-testcase/builds/77837020

[rogierslag]

I have found the main problem (at least for this I think). We load the specific table multiple times, but each time the polymorphous relation is set to another table. Squeel seems unable to handle this.

We have "fixed" the problem by using a :pluck query first, and use the result of that within squeel. It means a bit more overhead, but also gives us the possibility to upgrade to Rails 4.2.x

[gamov]

I'm trying to migrate an app to Rails 4.2 but I see a weird bug where A:Relation binding values are in the wrong order when compiled.
In my particular query, I'm adding a .where(attr: value) to an existing scope from a STI table which has other conditions on associations (wheres).
In RelationExtensions(4.2)#expand_attrs_from_hash, bind_values are added at the end of the array instead of beginning so when the DBStatement compiles the query, the wrong params are assigned to the wrong places, creating a nonsensical query that doesn't crash!
In the Rails 4.2.6 QueryMethods#L991, associations binds are added AFTER the targeted model table ones, maybe that's the reason (as @rogierslag pointed out).
Update: I removed Squeel and confirm that the bug is gone.

(Doesn't Squeel need fixing instead of Rails in this case?)

SQL Differences

# r4.2.6 w/ Squeel - Wrong (Note that the container_type inherited from the carrier_id value and dest_place_id from the container_type, etc...)
SELECT  `charges`.* FROM `charges` INNER JOIN `shipments` ON `shipments`.`id` = `charges`.`shipment_id` LEFT OUTER JOIN `shipment_bookings` ON `shipment_bookings`.`id` = `shipments`.`shipment_booking_id` 
WHERE `charges`.`type` IN ('ShipmentCharge') AND `shipment_bookings`.`etd` > '2013-06-20' AND `charges`.`container_type` = 1 AND (`shipment_bookings`.`carrier_id` = 20 AND `shipment_bookings`.`orig_place_id` = 2 AND `shipment_bookings`.`dest_place_id` = 0)

# r4.1.15 w/ Squeel - Correct
SELECT  `charges`.* FROM `charges` INNER JOIN `shipments` ON `shipments`.`id` = `charges`.`shipment_id` LEFT OUTER JOIN `shipment_bookings` ON `shipment_bookings`.`id` = `shipments`.`shipment_booking_id` 
WHERE `charges`.`type` IN ('ShipmentCharge') AND `shipment_bookings`.`etd` > '2013-06-20' AND `charges`.`container_type` = 0 AND ((`shipment_bookings`.`carrier_id` = 1 AND `shipment_bookings`.`orig_place_id` = 20 AND `shipment_bookings`.`dest_place_id` = 2))  

# r4.2.6/r4.1.15 w/o Squeel - Correct
SELECT  `charges`.* FROM `charges` INNER JOIN `shipments` ON `shipments`.`id` = `charges`.`shipment_id` LEFT OUTER JOIN `shipment_bookings` ON `shipment_bookings`.`id` = `shipments`.`shipment_booking_id` 
WHERE `charges`.`type` IN ('ShipmentCharge') AND `shipment_bookings`.`carrier_id` = 1 AND `shipment_bookings`.`orig_place_id` = 20 AND `shipment_bookings`.`dest_place_id` = 2 AND (`shipment_bookings`.`etd` > '2013-06-20') AND `charges`.`container_type` = 0