Problem with aliases on paginated fields
wmdmark opened this issue · 14 comments
I'm running into an issue where I have a paginated field with filter arguments that is being aliased to produce two different result trees. The problem is that both result trees end up coming back with the same result (whatever the last alias resolves to).
I can replicate this issue on the pagination demo as well.
Here's my query:
{
school {
privateCourses: resources(first: 1, visibility: private) {
edges {
node {
id
}
}
}
publicCourses: resources(first: 1, visibility: visible) {
edges {
node {
id
}
}
}
}
}When this comes back, both privateCourses and publicCourses are set to the same result (the latter).
{
"data": {
"school": {
"privateCourses": {
"edges": [
{
"node": {
"id": 18169
}
}
]
},
"publicCourses": {
"edges": [
{
"node": {
"id": 18169
}
}
]
}
}
}
}The query join monster generates looks like this:
SELECT
"school"."id" AS "id",
"resources$"."id" AS "resources$__id",
"resources$"."$total" AS "resources$__$total"
FROM school_school "school"
LEFT JOIN LATERAL (
SELECT *, count(*) OVER () AS "$total"
FROM course_course "resources$"
WHERE "resources$".school_id = "school".id AND "resources$".archived_time is NULL AND "resources$".highest_visibility_level >= 10
ORDER BY "resources$"."created" DESC
LIMIT 2 OFFSET 0
) "resources$" ON "resources$".school_id = "school".id
WHERE "school".id = 1
ORDER BY "resources$"."created" DESCHere's the relevant debug output in case it helps:
join-monster SQL_AST
join-monster { type: 'table',
name: 'school_school',
as: 'school',
children:
[ { type: 'column', name: 'id', fieldName: 'id', as: 'id' },
{ args: { first: 1, visibility: 'visible' },
paginate: true,
orderBy: { created: 'desc' },
type: 'table',
name: 'course_course',
as: 'resources$',
children:
[ { type: 'column', name: 'id', fieldName: 'id', as: 'id' },
{ type: 'column',
name: '$total',
fieldName: '$total',
as: '$total' },
{ type: 'column', name: 'id', fieldName: 'id', as: 'id' },
{ type: 'column', name: 'id', fieldName: 'id', as: 'id' },
{ type: 'column',
name: '$total',
fieldName: '$total',
as: '$total' },
{ type: 'column', name: 'id', fieldName: 'id', as: 'id' },
{ type: 'columnDeps', names: {} } ],
fieldName: 'resources',
grabMany: true,
where: [Function: where],
sqlJoin: [Function: sqlJoin] },
{ type: 'columnDeps', names: {} } ],
fieldName: 'school',
grabMany: false,
where: [Function: where] } +26ms
join-monster
join-monster SQL
join-monster SELECT
"school"."id" AS "id",
"resources$"."id" AS "resources$__id",
"resources$"."$total" AS "resources$__$total"
FROM school_school "school"
LEFT JOIN LATERAL (
SELECT *, count(*) OVER () AS "$total"
FROM course_course "resources$"
WHERE "resources$".school_id = "school".id AND "resources$".archived_time is NULL AND "resources$".highest_visibility_level >= 10
ORDER BY "resources$"."created" DESC
LIMIT 2 OFFSET 0
) "resources$" ON "resources$".school_id = "school".id
WHERE "school".id = 1
ORDER BY "resources$"."created" DESC +2ms
@wmdmark Ah I see. This is a bug indeed. I did an optimization that allows calls to the same field to be merged together. However, I forgot to diff the arguments before attempting to merge the fields. The second call to resources therefore overwrites the first one's args.
@acarl005 just checking to see what the status of this one is. I know you're probably super busy so I'm happy to help if you can point me in the right direction. Thanks for all your work on join monster, it's been working great for us so far!
This is actually a pretty rough fix. One of the fundamental assumptions made from the beginning is problematic and is blocking progress. The assumption is that the hydrated data will be placed on an object property with the same name as the field name. This is so graphql's default resolvers can get the data. However, there are two fields with the same name and different arguments (and hence different data). Only one of the fields' results can be assigned to the resources property in the hydrated data.
In order to fix this, Join Monster can no longer depend on the default resolver. The two (different) results from that field must be retrieved by a custom resolver, and each invocation of the resolver needs to know what the right property name is in the hydrated data, as it can no longer be assumed to be the field name.
I cannot estimate how long fixing this would take, as Join Monster would need to be re-thought.
Perhaps you can work around it ?
{
school {
# make private resources and public resources separate fields
privateResources(first: 1) {
edges {
node {
id
}
}
}
publicResources(first: 1) {
edges {
node {
id
}
}
}
}
}This may be repetitive, but it should work.
Thanks for the update. Bummer that full support for GraphQL aliases isn't possible but I understand the dilemma.
Interestingly, when I moved the resources field to the root query, the aliases worked fine so that solves my issue for now.
A fix for this is now possible. Facebook's graphql@v0.10 recently came out and it introduced the ability to overwrite the default resolver. Join Monster can provide its own default resolver to address this issue. Having an alternative default resolver enables Join Monster to guarantee uniqueness of property names when hydrating the data, for example, by using the alias names rather than the field names.
Such a change will incur a slight increase in configuration overhead and break interface. It will also need graphql-js 0.10 or later. This fix will therefore have to wait for Join Monster v2, which is currently in the works
I've debug it for 10 hours, unfortunately I can't figure out how to resolve field with alias name instead of original field name, I were able to fix the query handling and getting the needed data in a perfect way but then the response coming with null instead of taking the data from aliased names on data object.
Any expectations about this bug resolution? Let's put some effort together and figure it out, it's really annoying bug :(
We make heavy use of filter params for our GQL connections, so having the fields resolved with the alias name would help a lot.
Adding more fields on the schema level just to filter our data is a bummer.
still not fixed :( version 2.0.16
I've been thinking about this and technically these are two queries and thus should result in two database queries. When this happens, perhaps we can spawn some kind of "Super SQL AST node" indicating an entrypoint into a new query.
We should discuss more about the solution so if anyone can help they can take a look!
The problem doesn't seem to be specific to paginated queries, it also happens when you do not paginate. Could someone fix the title of the issue?
Hello there. This is an issue that I think many of us are still seeing unfortunately. By the sound of it, it looks like a pretty difficult issue. I took a peek and can see why.
What are your thoughts on adding a fix that requires a custom resolver? Even if its just to support aliases. I don't know if it covers all of the edge-cases or if its the best solution, but its one way that seemed to help support this.
The one thing that I am kind of concerned about by not addressing this bug is that the issue is 'hidden'. Aliases are a pretty normal use case. I know at least a few of us have spent quite a bit of time debugging our queries before finding that's its a bug here.
I haven't quite got the project building yet locally, but I can help look into this if needed. There is also a second issue open at #481 that has a pretty similar solution to where I was headed.
Thanks!