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

Querying the same relation multiple times as aliases with different args results in wrong data #481

Closed
kryops opened this issue May 9, 2022 · 5 comments

Comments

@kryops
Copy link

kryops commented May 9, 2022

Description

Hi there,

in a GraphQL query that uses aliases to query the same relation multiple times with different args, the relation is resolved only once, which results in wrong results for the other aliases.

Reproduction

table1 has a 1:1 or 1:n relation to table2 that uses args:

{
  table2: {
    type: Table2Type,
    args: {
      id: { type: new GraphQLNonNull(GraphQLID) },
    },
    extensions: {
      joinMonster: {
        sqlJoin: (currentTable, targetTable, args) => `${targetTable}.id = '${escapeSqlString(args.id)}'`,
      },
    },
  },
}

Now I query the same relation twice with different args:

{
  table1 {
    foo: table2(id: "1") { id }
    bar: table2(id: "2") { id }
  }
}

Expected Result:

{
  table1: {
    foo: { id: "1" }
    bar: { id: "2" }
  }
}

Actual Result:

{
  table1: {
    foo: { id: "2" }
    bar: { id: "2" }
  }
}

I did a bit of debugging, and these are my findings:

  • with sqlJoin, the first relation is not even joined - handleSelections recognizes it as existingNode and overrides it with the second one
  • with sqlBatch, the SQL queries are executed as expected, but the last result seems to overwrite the former ones when the results are stitched back together

The root issue seems to be that the GraphQL default resolver (that gets passed down the result from join-monster) expects the original fieldName as object property, and GraphQL handles the aliasing somewhere internally: https://github.com/graphql/graphql-js/blob/97c95124b17ec42b62917a65c94fd528c69c7cfe/src/execution/execute.ts#L1015.

Thus, join-monster ignores the aliases and uses the original field names, which can lead to conflicts in this case.

This is the result object that GraphQL currently receives from join-monster:

{
  table1: {
    table2: { id: "2" }
  }
}

In order to make this use case work, it might have to look something like this:

{
  table1: {
    table2$foo: { id: "1" }
    table2$bar: { id: "2" }
    table2: function(args, context, info) {
      // return the correct property depending on info.fieldNodes[0].alias.value
    }
  }
}

Is this something that can be fixed in join-monster? Is there a way to work around it?

Thanks for your support!

Environment

  • join-monster@3.1.1
  • graphql@15.8.0
@kryops
Copy link
Author

kryops commented May 12, 2022

I did some experiments in a fork, and stumbled upon the skipped test it should handle duplicate object type fields with different arguments that should cover this case. I added my own test in kryops@d6e1a26

The best I could come up with for now is a workaround: kryops@f75a574

  • If a field has an alias, it is not only written to row[fieldName], but also to row[fieldName + '$' + alias]
  • Fields with args have to set resolve: aliasAwareResolver, which prefers the aliased key over the default one
  • I did not look at pagination as we use SQLite which currently does not support it

While this is still far from a viable solution, maybe it could be used as a starting point for a proper fix?

@kryops
Copy link
Author

kryops commented May 12, 2022

Ah, I finally found #126, so it seems I created a duplicate 😕

@neilgaietto
Copy link

I just recently ran into this issue as well. Its surprising that its a bug that's been around for years because it seems like its such a common use case. I can assist where needed to help get this fix out.

@kryops
Copy link
Author

kryops commented May 12, 2022

While my workaround seems to work quite well for 1:n relationships, it seems to fail miserably for aliased 1:1 relationships because @stem/nesthydrationjs does not like hydrating the same source fields into multiple nested targets - instead it just sets everything to null 😞

const { nest } = require('@stem/nesthydrationjs');

console.log(nest(
  { id: '1', foo: 'bar' },
  {
    id: 'id',
    child: { foo: 'foo' },
    child$alias: { foo: 'foo' },
  }
));

// { id: '1', child: null, 'child$alias': null }

@kryops
Copy link
Author

kryops commented May 16, 2022

I am closing this in favor of #126

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants