Graphql-ruby: [pro] Can't use `cursor` or `endCursor` on a connection field which has a join in the relation

[cheerfulstoic]

I'm getting the error:

Failed to build GraphQL cursor, no attribute `cursor_0`

It trackes back to graphql-pro-1.9.3/lib/graphql/pro/ordered_relation.rb:23

I think it's because the select happening below doesn't get picked up by ActiveRecord when there are joins because ActiveRecord defines it's own columns and so only uses those to load attributes into the model.

Instead, couldn't it just access node.attributes[v.name] (or perhaps node.send(v.name) in case there's some sort of mapping)? That would also mean that you wouldn't need to send an extra column's worth of data back from the DB

[cheerfulstoic]

Working on creating a repo that reproduces this, but as I was doing that I found that more specifically it fails because there is both a join and an includes in the relation. That's what causes Rails to define it's own set of individual columns for the multiple tables it's joining on (a join by itself still just does table_name.*)

[rmosolgo]

Hey, thanks for reporting this! One trick to consider is that, when a relation is sorted by a SQL function, we also need to select the value of that function in order to generate the cursor. (The other option would be to reverse-engineer the function and implement it in ruby, using the attributes as input).

But, maybe a hybrid solution would work in this case, reading from attributes when that is sufficient, but using extra .selects when necessary.

Could you share an example relation that demonstrates this bug? (Maybe not real code, but just something that could inform my bug hunting! 🔍 )

[cheerfulstoic]

:+1: I didn't think about that (though I figured there was probably something I hadn't thought of ;) ).

I've made an example app to show the issue. I was playing around with something else as well in there, but I'll give the info on how to reproduce this bug too. The app:

https://github.com/cheerfulstoic/graphql_pro_bug_example_app

Of course you'll need to run the bundle config for the graphql-pro key. In that app I can reproduce the issue with the following query:

query {
  piratesCursorIssue(first: 1) {
    pageInfo {
      endCursor
      hasNextPage
    }
    edges { cursor node { id name age } }
  }

You can see that the implementation of pirates_cursor_issue is just Pirate.joins(:ships).includes(:ships), though in my app I noticed it was actually occurring just with an includes, though I think that particular inclues might have been set up strangely to cause ActiveRecord to do a JOIN in SQL. But I think a fix to this would fix the other.

[cheerfulstoic]

Oh, and I forgot to mention that you should do a rake db:seed in addition to the regular database setup so that the objects are there.

[rmosolgo]

Thanks for sharing that, I'll take a look soon!

[rmosolgo]

Hey, thanks again for your help hunting this down. I spent some time with your example to understand the problem better.

I see that graphql-pro's cursor information _is_ added to the SELECT clause:

  SQL (2.6ms)  SELECT pirates.*, pirates.id AS cursor_0, "pirates"."id" AS t0_r0, "pirates"."name" AS t0_r1, "pirates"."age" AS t0_r2, "pirates"."created_at" AS t0_r3, "pirates"."updated_at" AS t0_r4, "ships"."id" AS t1_r0, "ships"."name" AS t1_r1, "ships"."pirate_id" AS t1_r2, "ships"."created_at" AS t1_r3, "ships"."updated_at" AS t1_r4 FROM "pirates" INNER JOIN "ships" ON "ships"."pirate_id" = "pirates"."id" WHERE (age > 30) AND "pirates"."id" = 2 ORDER BY pirates.id asc
                                 ^^^^^^^^^^^^^^^^^^^^^^

But, by the time ActiveRecord has initialized a record based on the SQL results, that selection is gone:

[20, 29] in /Users/rmosolgo/.rbenv/versions/2.5.1/lib/ruby/gems/2.5.0/gems/graphql-pro-1.9.3/lib/graphql/pro/ordered_relation.rb
   20:       # @param node [Object] a member of the relation
   21:       # @return [String] GraphQL-ready cursor for `node`
   22:       def build_cursor(node)
   23:         JSON.dump(@order_values.each_with_index.map { |v, idx|
   24:           byebug
=> 25:           if !node.attributes.key?("cursor_#{idx}")
   26:             raise("Failed to build GraphQL cursor, no attribute `cursor_#{idx}`")
   27:           else
   28:             node.attributes["cursor_#{idx}"]
   29:           end
(byebug) node.attributes.keys
["id", "name", "age", "created_at", "updated_at"]

(The code there expects cursor_0 to exist, since it was previously added to the SELECT.)

So, it seems like when Rails breaks a selection down into t0_r0 ... etc, it initializes a record using _only_ those selections. The issue can be seen in isolation here:

(byebug) Pirate.all.select("pirates.id as pirate_id").joins(:ships).includes(:ships).first.attributes
  SQL (0.5ms)  SELECT  DISTINCT "pirates"."id" FROM "pirates" INNER JOIN "ships" ON "ships"."pirate_id" = "pirates"."id" ORDER BY "pirates"."id" ASC LIMIT ?  [["LIMIT", 1]]
  SQL (0.2ms)  SELECT pirates.id as pirate_id, "pirates"."id" AS t0_r0, "pirates"."name" AS t0_r1, "pirates"."age" AS t0_r2, "pirates"."created_at" AS t0_r3, "pirates"."updated_at" AS t0_r4, "ships"."id" AS t1_r0, "ships"."name" AS t1_r1, "ships"."pirate_id" AS t1_r2, "ships"."created_at" AS t1_r3, "ships"."updated_at" AS t1_r4 FROM "pirates" INNER JOIN "ships" ON "ships"."pirate_id" = "pirates"."id" WHERE "pirates"."id" = 1 ORDER BY "pirates"."id" ASC
{"id"=>1, "name"=>"Black Belly", "age"=>29, "created_at"=>Mon, 07 Jan 2019 14:44:08 UTC +00:00, "updated_at"=>Mon, 07 Jan 2019 14:44:08 UTC +00:00}

Although I added .select("pirates.id as pirate_id"), There was no "pirate_id"=> in the loaded model's attributes.

So... what can we do..?

• I found that cursors worked ok with _either_ .joins _or_ .includes, is it possible to implement the field with only one of those?
• Hunt the issue with Rails. I found some similar ones(https://github.com/rails/rails/issues/12423, https://github.com/rails/rails/issues/15185), but nothing identical, so I opened an issue: https://github.com/rails/rails/issues/34889
• Reimplement cursor value tracking in some way. Currently, it uses SELECT ... to turn order values into attributes of models. But it would Well, I was going to say, it would be possible to implement this using _Ruby_ instead, to convert order values into Ruby method calls (eg ORDER BY id -> .id), but this won't work with any complex orders. I'm not sure about it.

[cheerfulstoic]

With regard to implementing it without both joins and includes... maybe.. For context: I have helper methods in my GraphQL schema classes which automatically do includes on associations which have been requested in the GraphQL query by looking at the ast_node. In the field in question I'm using joins to filter out records according to it's association's details. So at the moment I'm working around it with an ugly hack which ignores the specific association which is being automatically eager loaded.

But I suspect you might actually see this problem using just includes in certain cases. Rails might have changed more recently, but in the past if you do an includes and then reference the associated table it makes one query rather than two and falls back to selecting the t0_r0 ... fields that you're seeing. The documentation for the includes method seems to imply that you need to call references to indicate that it needs to do that join. So probably the magic condition parsing that they were doing previously is gone, but the use-case (people calling references) is likely still there.

So I don't know that I know what a solution might be, but I've subscribed to https://github.com/rails/rails/issues/34889 and I look forward to hearing what the Rails team has to say

[cheerfulstoic]

;( I've found another issue with the cursor_0. I have a field with a method on the UserType class which is defined like:

def field
  Model.recent(object)
end

That recent class method is defined as:

def recent(user)
  Model.joins(:other_model).order(gmtDate: :desc).where(other_model_table: { owner: user }).distinct
end

So basically it's supposed to return a distinct list. But it doesn't because the query is like this:

SELECT
DISTINCT model_table.*,
other_model_table.gmtDate AS cursor_0,
model_table.id AS cursor_1
FROM `model_table` INNER JOIN `other_model_table` ON `other_model_table`.`model_id` = `model_table`.`id`
WHERE `other_model_table`.`owner` = 15
ORDER BY `model_table`.`gmtDate` DESC, model_table.id asc
LIMIT 51

So because it's bringing cursor_0 into the query the DISTINCT doesn't see a difference between the rows because the gmtDate is part of the result.

I still don't (yet) know of a solution, but adding a cursor to the queries is seeming more and more to me like a thing that is just going to bite people in various ways.

[rmosolgo]

A fix to the underlying Rails issue was just merged into Rails master 🍻

[cheerfulstoic]

🎉 I saw! Since it's a bug, do you think it will come out in the next patch release?

[rmosolgo]

Yes, I would _expect_ so, I'll keep an eye on the release notes and follow up here!

[qrush]

Big +1 to getting a patch in for the next release. 🙌

[rmosolgo]

👋 I've just released graphql-pro 1.12.x which offers another approach to this issue. In GraphQL-Ruby 1.10.x (also just released), you can apply connection wrappers _inside the field resolver_ which gives you much more flexible control over which list objects get which connection behaviors.

So, with graphql-pro 1.12.x (and graphql-ruby 1.10.x), you could enable the new stable relation connection (https://graphql-ruby.org/pagination/stable_relation_connections.html) and then _opt out_ on a field-by-field basis, falling back to index-based cursors when a stable cursor can't be created:

def items 
  items = Item.joins(...).select(...) # etc 
  # apply an index-based connection in this specific case:
  GraphQL::Pagination::ActiveRecordRelationConnection.new(items)
end 

Let's keep an eye on the upstream fix, too, but I thought I'd share this solution for now!

[rmosolgo]

It looks like the fix is due on the next release of Rails 6 (it's on the 6-0-stable changelog, https://github.com/rails/rails/blob/6-0-stable/activerecord/CHANGELOG.md), so I don't have any more work planned here. If this pops up again after you update the the next Rails version, please open a new issue!