Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Throw error when pagination with after #97

Closed
trungtin opened this issue Oct 22, 2017 · 3 comments
Closed

Throw error when pagination with after #97

trungtin opened this issue Oct 22, 2017 · 3 comments

Comments

@trungtin
Copy link

https://github.com/graphile/graphile-build/blob/db30a95d80b5eca17b78cdedda63b778b9463c37/packages/graphile-build-pg/src/queryFromResolveData.js#L135-L137

With this simple query:

query getMyTasks($taskCondition: TaskCondition!) {
  allTasks(condition:$taskCondition, after:"WyJ0YXNrcyIsIndkTHdiIl0=") {
    totalCount
    nodes {
      name
      nodeId
      id
    }
  }
}

It will throw Error because cursorValue[getPgCursorPrefix().length] is (id) not [(id)].
Example cursorValue at this point is:

['tasks', 'wdLwb']

Simply change to:

const cursor = cursorValue[getPgCursorPrefix().length]
const sqlCursors = (Array.isArray(cursor) ? cursor : [cursor]).map(val =>
  sql.value(val)
);

work though. Have no idea whether I have set thing up wrongly or anything else.

@benjie
Copy link
Member

benjie commented Oct 23, 2017

Ah yeah, this is likely due to a compatibility hack I had to do to make the cursors line up between v4 and v3. Cursors come from:

https://github.com/graphile/graphile-build/blob/731d2239c1da47a60765ba64cd55cefe71f085fe/packages/graphile-build-pg/src/queryFromResolveData.js#L115-L128

It's line 126 above that's introducing the issue I think because the value is not a (nested) array (unlike line 118 which is) - this is the compat hack I had to do - without it I broke a lot of acceptance tests.

However, I would not expect this to hit that line because there should be an implicit orderBy in the query you're running. If you add an explicit orderBy does the problem go away? I'm wondering if you've not defined a primary key on the tasks table? This is still a bug if so, but it helps explain why it's occurring and why it wasn't caught earlier. If this isn't the case then I'm somewhat confused as to what has caused it.

I'd encourage a PR; in it I'd want to see:

@benjie
Copy link
Member

benjie commented Oct 28, 2017

I'm hoping this has been solved by a combination of #107 and #101

@benjie
Copy link
Member

benjie commented Oct 28, 2017

[semi-automated message] To keep things manageable I'm going to close this issue as I think it's solved; but if not or you require further help please re-open it.

@benjie benjie closed this as completed Oct 28, 2017
@benjie benjie added this to the 4.0 milestone Aug 16, 2018
madtibo pushed a commit to spacefill/graphile-engine that referenced this issue Dec 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants