ActiveRecord::StatementInvalid errors with v1.0.0 and Rails 5

[xtagon]

Hi,

I'm trying to use v1.0.0 with Rails 5, and my queries don't quite work. I could really use some help.

This does what is expected (gets the ancestors as a relation):

relation = self.class.join_recursive do |query|
  query.start_with(id: id).connect_by(parent_organization_id: :id)
end

and if I call #to_a on that, I get the correct results. However, if I do any of these things:

relation.first, relation.where(...), relation.limit(...), relation.order(..) I get an error like this:

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

Here's an example of the generated SQL when it fails to on relation.first:

SELECT "organizations".* FROM "organizations"
INNER JOIN
  (WITH RECURSIVE "organizations__recursive" AS
     (SELECT "organizations"."id", "organizations"."parent_organization_id"
      FROM "organizations"
      WHERE "organizations"."deleted_at" IS NULL
        AND "organizations"."id" = $1
        UNION ALL
        SELECT "organizations"."id",
               "organizations"."parent_organization_id"
        FROM "organizations"
        INNER JOIN "organizations__recursive" ON "organizations__recursive"."parent_organization_id" = "organizations"."id" WHERE "organizations"."deleted_at" IS NULL ) SELECT "organizations__recursive".*
   FROM "organizations__recursive") AS "organizations__recursive" ON "organizations"."id" = "organizations__recursive"."id"
WHERE "organizations"."deleted_at" IS NULL
ORDER BY "organizations"."id" ASC
LIMIT $2

Any ideas? Thanks in advance!

[thooams]

It's start_with method which generates an error. I don't know why.
Only variable $2 (LIMIT) is taken into account.

[thooams]

Try to replace start_with(id: id) by start_with(YourModel.find(id)) or start_with(self)

[mkamensky]

I see the same problem. If I try to give the object as an argument to start_with, it ignores the condition

[mkamensky]

It appears that when chaining with other queries, the argument list gets replaced

[mkamensky]

A workaround similar to the one mention in rails/rails#20077 (comment) worked for me

[zachaysan]

Is this still an issue in Rails 5.1? I'm going to help keep this project maintained and I want to fix what's broken and make sure we can support Rails 5.2 now that it is out.

[xtagon]

@zachaysan I would suspect so. One of the reasons I'm still stuck on Rails 4.2 is because I couldn't upgrade activerecord-hierarchical_query without issues like this when attempting a Rails 5 upgrade, so if you're looking to help fix and maintain, I would be happy to help you and do testing and try a Rails 5 upgrade again. Not sure on the time frame though due to work responsibilities.

[zachaysan]

Great @xtagon I don't want you to waste time so if you could just give me a detailed, but austere way of replicating the bug I'll try to handle as soon as possible.

[xtagon]

@zachaysan I'll have to pull my Rails 5 upgrade branch and see if I can reproduce. There have been a lot of changes to my own app since I last tried the upgrade. I'll try to do it this weekend. Thanks so much for the help!

[zachaysan]

Hey @xtagon I'm going to close this issue. If you can re-produce in 1.1.0 let me know, but I'm currently unable to.

[xtagon]

Sounds fair. It is going to take me a little longer to reproduce again as I'm fixing some conflicts in the branch I was working in. I'll re-open the issue when it gets to that point.

[xtagon]

@zachaysan Just to let you know, I reproduced this on 1.1.0 and Rails 5.0, but this time when I changed start_with to use self instead of the model ID, it fixed it. Awesome!

Thanks again for everyone's help.

[zachaysan]

@xtagon Dang really? What happens if you do self.id?

[xtagon]

If I pass the ID it has a prepared statement error like the one I originally shared. If I pass the model object it works. I think if I pass an array of multiple IDs it works too.

[zachaysan]

I'm trying to make sure it's not related to some id weirdness. Sometimes ids are strings in rails and sometimes they're integers.

What happens if you do this:

relation = self.class.join_recursive do |query|
  query.start_with(id: self.id).connect_by(parent_organization_id: :id)
end

[xtagon]

Here is part of the code that I got the error from:

relation = self.class.join_recursive do |query|
      query.
        start_with(id: self.id) { select("0 depth") }.
        select(query.prior[:depth] + 1, start_with: false).
        connect_by(id: :parent_organization_id)
    end

It only causes an error if you do a where condition or similar later on this relation as described above. If you do a to_a directly on it it's fine. Replacing self.id with self fixes it too.

I didn't think of this before but I am using Postgres GUID strings as IDs if that helps.

[zachaysan]

Hmm. I'm going to bet that GUID is why I can't seem to replicate because I'm not currently using them and they're another tricky case of sometimes integer, sometimes string tomfoolery.

Are you able to clone the repo and run the tests? Could you create a failing tests with a GUID column? If you can do that I'll try to figure out the root cause.

[xtagon]

I'll give it a shot.

[xtagon]

All tests pass when I switch your test schema to use UUIDs. I even added a test case that looks as close as possible to the one in my app (I am not able to give access to my app's source unfortunately).

Very strange, I'm not sure why it won't reproduce.

xtagon@4ea5b6e

I made sure to use the Rails 5.0 gemfile too.

[zachaysan]

Hmmm. Are you using any weird default scopes or redefining any core methods?

[xtagon]

I am using this soft deletion gem on the model, which adds a default scope for deleted_at IS NULL: https://rubygems.org/gems/soft_deletion

[zachaysan]

Sorry for the delay. Does disabling the gem do anything? I dislike uncertainty and spookiness in libraries so I'd like to figure out what is wrong.

[xtagon]

Just tried that, it didn't seem to have an effect.

This is on Rails 5.0.0 - I'm going to be upgrading to 5.1 and then 5.2 soon-ish, maybe it was something that changed in Arel

[xtagon]

I'd like to figure out what is wrong too, but just to clarify I'm perfectly fine with the workaround of using the record instance -- it doesn't solve the mystery but it solves my use case

[zachaysan]

Well if it is still an issue in 5.2 then I'm going to try to figure it out. Partially for my own interests. I want to be able to rely on this library in production and I generate some pretty abstract queries during a request cycle so this needs to be dependable.