[PRO] cursor-based pagination fails with whitespace after order

[jscheid]

Describe the bug

Cursor-based pagination fails when there is whitespace after an order literal, see repro.

Versions

graphql version: 2.0.16 rails (or other framework): 6.0.6.1 other applicable versions (graphql-batch, etc): graphql-pro 1.23.9

GraphQL schema

N/A

GraphQL query

N/A

Steps to reproduce

Enable cursor-based pagination and try using a connection on a relation looking like this:

MyModel.order(Arel.sql('foo DESC     '))

Expected behavior

No error, same behavior as without whitespace.

Actual behavior

Invalid cursor for `after`. Fetch a new cursor and try again.

Additional context

Note that this bug masquerades as: Invalid cursor for `after`. Fetch a new cursor and try again. because it corrupts the statement, and any ActiveRecord::StatementInvalid error gets turned into that error message. Perhaps that part of the code could be improved to preserve reporting of genuinely invalid statements.

[rmosolgo]

Hmmm, sorry for the trouble!

I'm torn because, on the one hand, you'd want to provide the nicest error, just like you've described here. But on the other hand, I don't know of a way to do that. StatementInvalid comes from trying to run the database query, and the database rejecting it. Should GraphQL try to run the unbounded query in order to validate it? I don't think so... And I can't find a way for ActiveRecord to validate SQL without running it. (I thought maybe explain, but the docs say that explain may actually run queries.)

So, I'm not sure how to validate the relation provided by the application here, without running it.

Another approach would be to let ActiveRecord::StatementInvalid propagate up in some cases. But which cases? Maybe, when it happens, GraphQL could try running the original relation to see whether it also raises the error, and if it does, then let that error cascade up.

That approach might work, I'll investigate it and follow up here!

[jscheid]

Do I understand correctly that this is meant to catch cases where there's a type mismatch between the cursor column and the cursor value passed in by the client?

[jscheid]

@rmosolgo also, just checking that you saw this issue isn't primarily about error reporting, but about a specific bug to do with whitespace handling?