Skip to content

fix(GH-220): Cannot nest multiple queries on the same table #222

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

Merged

Conversation

igdianov
Copy link
Collaborator

@igdianov igdianov commented Dec 10, 2019

This PR fixes the query and mapping problem with nested recursive queries in where search criterias with @ManyToMany mapping on Character entity.

For example, when mapping "author" / "book" as many-to-many, we need "authors" collection on Book and "books" collection on Author. In our case, "Character" entity represents both ends of a relationship; so we need "friends to" and "friend of" collections.

We can still use the same association table, but note that friendsOf is mappedBy friends.

@Entity
@GraphQLDescription("Abstract representation of an entity in the Star Wars Universe")
@Getter
@Setter
@ToString
@EqualsAndHashCode(exclude={"appearsIn","friends", "friendsOf"}) // Fixes NPE in Hibernate when initializing loaded collections #1
public abstract class Character {

    @Id
    @GraphQLDescription("Primary Key for the Character Class")
    String id;

    @GraphQLDescription("Name of the character")
    String name;

    @GraphQLDescription("Who are the known friends to this character")
    @ManyToMany(fetch=FetchType.LAZY)
    @JoinTable(name="character_friends",
            joinColumns=@JoinColumn(name="source_id", referencedColumnName="id"),
            inverseJoinColumns=@JoinColumn(name="friend_id", referencedColumnName="id"))
    @OrderBy("name ASC")
    Set<Character> friends; 

    @GraphQLDescription("Who are the known friends of this character")
    @ManyToMany(fetch=FetchType.LAZY, mappedBy = "friends")
    @OrderBy("name ASC")
    Set<Character> friendsOf;     
    
    @GraphQLDescription("What Star Wars episodes does this character appear in")
    @ElementCollection(targetClass = Episode.class, fetch=FetchType.LAZY)
    @Enumerated(EnumType.ORDINAL)
    @OrderBy
    Set<Episode> appearsIn;
    
    Character() {}

}

The "friends" and "friendsOf" collections may or may not match (depending on whether "friendship" is mutual or not).

After fixing the relationship, the following query returns correct result:

query {
  Droids {
     select {
      name
      friends(where: {friendsOf:{name: {EQ: "Luke Skywalker"}}}) {
        name
        friendsOf {
          name
        }
      }
    }
  }
}

Generated JPQL:

select distinct droid from Droid as droid 
left join fetch droid.friends as generatedAlias0 
left join fetch generatedAlias0.friendsOf as generatedAlias1 
where generatedAlias1.name='Luke Skywalker'
order by droid.id asc

Result:

{
  "data": {
    "Droids": {
      "select": [
        {
          "name": "C-3PO",
          "friends": [
            {
              "name": "Han Solo",
              "friendsOf": [
                {
                  "name": "Luke Skywalker"
                }
              ]
            },
            {
              "name": "Leia Organa",
              "friendsOf": [
                {
                  "name": "Luke Skywalker"
                }
              ]
            },
            {
              "name": "R2-D2",
              "friendsOf": [
                {
                  "name": "Luke Skywalker"
                }
              ]
            }
          ]
        },
        {
          "name": "R2-D2",
          "friends": [
            {
              "name": "Han Solo",
              "friendsOf": [
                {
                  "name": "Luke Skywalker"
                }
              ]
            },
            {
              "name": "Leia Organa",
              "friendsOf": [
                {
                  "name": "Luke Skywalker"
                }
              ]
            }
          ]
        }
      ]
    }
  }
}

In the process, I have realized that regular root where expressions were adding extra joins instead of reusing existing fetch joins from the selection graph. I have fixed that and also enabled EXISTS predicate expressions on all relations to provide support for the previous behavior using subsquery expressions, i.e.

with WHERE

   @Test
    public void queryAuthorBooksWithExplictOptional() {
        //given
        String query = "query { "
                + "Authors(" + 
                "    where: {" + 
                "      books: {" + 
                "        title: {LIKE: \"War\"}" + 
                "      }" + 
                "    }" + 
                "  ) {" + 
                "    select {" + 
                "      id" + 
                "      name" + 
                "      books(optional: true) {" + 
                "        id" + 
                "        title(orderBy: ASC)" + 
                "        genre" + 
                "      }" + 
                "    }" + 
                "  }"
                + "}";
        
        String expected = "{Authors={select=["
                + "{id=1, name=Leo Tolstoy, books=["
                + "{id=2, title=War and Peace, genre=NOVEL}]}"
                + "]}}";

        //when
        Object result = executor.execute(query).getData();

        // then
        assertThat(result.toString()).isEqualTo(expected);
    }

With WHERE EXISTS

   @Test
    public void queryAuthorBooksWithExplictOptionalEXISTS() {
        //given
        String query = "query { "
                + "Authors(" + 
                "    where: {" +
                "      EXISTS: {" +
                "        books: {" + 
                "          title: {LIKE: \"War\"}" + 
                "        }" + 
                "      }" + 
                "    }" + 
                "  ) {" + 
                "    select {" + 
                "      id" + 
                "      name" + 
                "      books(optional: true) {" + 
                "        id" + 
                "        title(orderBy: ASC)" + 
                "        genre" + 
                "      }" + 
                "    }" + 
                "  }"
                + "}";
        
        String expected = "{Authors={select=["
                + "{id=1, name=Leo Tolstoy, books=[" 
                +    "{id=3, title=Anna Karenina, genre=NOVEL}, " +
                +    "{id=2, title=War and Peace, genre=NOVEL}]}"
                + "]}}";

        //when
        Object result = executor.execute(query).getData();

        // then
        assertThat(result.toString()).isEqualTo(expected);
    }    

It also fixes the problem with double wrapping of plural attribute types in GraphQL list type.

So, to summarize, all where root criteria expressions will now apply restrictions on the fetch joins. It will make results of the query filtered using joins and filter results in collections.

Count queries with apply restrictions on regular joins instead of fetch join for better performance.

There is expanded support to use EXISTS subquery for all relationship types to bridge for the old where criteria expressions. You can see changes in the tests

Fixes #220

@igdianov igdianov self-assigned this Dec 10, 2019
@igdianov igdianov added the bug label Dec 10, 2019
@codecov
Copy link

codecov bot commented Dec 10, 2019

Codecov Report

Merging #222 into master will decrease coverage by 0.14%.
The diff coverage is 95.83%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #222      +/-   ##
============================================
- Coverage     73.69%   73.54%   -0.15%     
+ Complexity      850      846       -4     
============================================
  Files            49       49              
  Lines          3668     3651      -17     
  Branches        613      606       -7     
============================================
- Hits           2703     2685      -18     
- Misses          700      703       +3     
+ Partials        265      263       -2
Impacted Files Coverage Δ Complexity Δ
...hql/jpa/query/schema/model/starwars/Character.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...hql/jpa/query/schema/impl/JpaPredicateBuilder.java 60.62% <100%> (+0.31%) 114 <1> (ø) ⬇️
...jpa/query/schema/impl/GraphQLJpaSchemaBuilder.java 91.43% <100%> (-0.17%) 113 <0> (-4)
...a/query/schema/impl/QraphQLJpaBaseDataFetcher.java 72.53% <100%> (-1.01%) 149 <10> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 963826f...24891c1. Read the comment docs.

@igdianov
Copy link
Collaborator Author

igdianov commented Dec 10, 2019

@molexx I have fixed the problem with nested multiple queries in where criterias. In the process, I have realized that regular root where expressions were adding extra joins instead of reusing existing fetch joins from the selection graph. I have fixed that and also enabled EXISTS predicates on all relations to provide support for the previous behavior using subsquery expressions.

So, to summarize, all where root criteria expressions will now apply restrictions on the fetch joins. It will make results of the query filtered using joins.

Count queries with apply restrictions on regular joins instead of fetch join for better performance.

There is expanded support to use EXISTS subquery for all relationship types to bridge for the old where criteria expressions. You can see changes in the tests.

As I mentioned in the issue, the many-to-many relationship with circular dependencies do not work correctly. I have added FIXME tests for those edge cases.

@molexx
Copy link
Contributor

molexx commented Dec 10, 2019

Thanks!

I think that makes sense to have EXISTS.

I'll pull this into my tree - along with graphql-13 and executor-context-factory.

Is the outstanding issue the one where Luke is appearing despite not being friends with himself? I think that is due to outer joins, I have made a comment on the issue.

@igdianov
Copy link
Collaborator Author

igdianov commented Dec 10, 2019

Is the outstanding issue the one where Luke is appearing despite not being friends with himself? I think that is due to outer joins, I have made a comment on the issue.

I cannot seem to find any solution to fix it with or without optional query modifier. Maybe you could see what is going on?

{
  Droids(where: {friends: {EXISTS: {friends: {name: {EQ: "Leia Organa"}}}}}) {
    select {
      name
      friends(optional: false) {
        name
        friends(optional: false) {
          name
        }
      }
    }
  }
}

or 

{
  Droids(where: {friends: {friends: {name: {EQ: "Leia Organa"}}}}) {
    select {
      name
      friends(optional: false) {
        name
        friends(optional: false) {
          name
        }
      }
    }
  }
}


JPQL Query Fetch Joins are inner

select distinct droid from Droid as droid 
inner join fetch droid.friends as generatedAlias0 
inner join fetch generatedAlias0.friends as generatedAlias1 
   where exists (select generatedAlias2 from generatedAlias0.friends as generatedAlias2 
where generatedAlias2.name=:param0) 
order by droid.id asc
Hibernate: 
    select
        droid0_.id as id2_0_0_,
        character2_.id as id2_0_1_,
        character4_.id as id2_0_2_,
        droid0_.name as name3_0_0_,
        droid0_.primary_function as primary_5_0_0_,
        character2_.name as name3_0_1_,
        character2_.primary_function as primary_5_0_1_,
        character2_.favorite_droid_id as favorite6_0_1_,
        character2_.gender_code_id as gender_c7_0_1_,
        character2_.homePlanet as homePlan4_0_1_,
        character2_.DTYPE as DTYPE1_0_1_,
        friends1_.source_id as source_i1_2_0__,
        friends1_.friend_id as friend_i2_2_0__,
        character4_.name as name3_0_2_,
        character4_.primary_function as primary_5_0_2_,
        character4_.favorite_droid_id as favorite6_0_2_,
        character4_.gender_code_id as gender_c7_0_2_,
        character4_.homePlanet as homePlan4_0_2_,
        character4_.DTYPE as DTYPE1_0_2_,
        friends3_.source_id as source_i1_2_1__,
        friends3_.friend_id as friend_i2_2_1__ 
    from
        Character droid0_ 
    inner join
        character_friends friends1_ 
            on droid0_.id=friends1_.source_id 
    inner join
        Character character2_ 
            on friends1_.friend_id=character2_.id 
    inner join
        character_friends friends3_ 
            on character2_.id=friends3_.source_id 
    inner join
        Character character4_ 
            on friends3_.friend_id=character4_.id 
    where
        droid0_.DTYPE='Droid' 
        and (
            exists (
                select
                    character6_.id 
                from
                    character_friends friends5_,
                    Character character6_ 
                where
                    character2_.id=friends5_.source_id 
                    and friends5_.friend_id=character6_.id 
                    and character6_.name=?
            )
        ) 
    order by
        droid0_.id asc,
        character2_.name asc,
        character4_.name asc

Result:

{
  "data": {
    "Droids": {
      "select": [
        {
          "name": "C-3PO",
          "friends": [
            {
              "name": "Han Solo",
              "friends": [
                {
                  "name": "Leia Organa"
                },
                {
                  "name": "Luke Skywalker"
                },
                {
                  "name": "R2-D2"
                }
              ]
            },
            {
              "name": "Luke Skywalker",
              "friends": [
                {
                  "name": "C-3PO"
                },
                {
                  "name": "Han Solo"
                },
                {
                  "name": "Leia Organa"
                },
                {
                  "name": "R2-D2"
                }
              ]
            },
            {
              "name": "R2-D2",
              "friends": [
                {
                  "name": "Han Solo"
                },
                {
                  "name": "Leia Organa"
                },
                {
                  "name": "Luke Skywalker"
                }
              ]
            }
          ]
        },
        {
          "name": "R2-D2",
          "friends": [
            {
              "name": "Han Solo",
              "friends": [
                {
                  "name": "Leia Organa"
                },
                {
                  "name": "Luke Skywalker"
                },
                {
                  "name": "R2-D2"
                }
              ]
            },
            {
              "name": "Leia Organa",
              "friends": [
                {
                  "name": "C-3PO"
                },
                {
                  "name": "Han Solo"
                },
                {
                  "name": "Luke Skywalker"
                },
                {
                  "name": "R2-D2"
                }
              ]
            },
            {
              "name": "Luke Skywalker",
              "friends": [
                {
                  "name": "C-3PO"
                },
                {
                  "name": "Han Solo"
                },
                {
                  "name": "Leia Organa"
                },
                {
                  "name": "R2-D2"
                }
              ]
            }
          ]
        }
      ]
    }
  }
}

@molexx
Copy link
Contributor

molexx commented Dec 10, 2019

@igdianov can you see my latest comment on the issue please? Executing the query myself it looks ok, the correct data is returned - 5 rows, 3 for C3PO and 2 for R2 - and Luke isn't listed as one of R2's friends yet appears in the graphql response, where is he coming from?

@igdianov igdianov force-pushed the fix-GH-220-cannot-nest-multiple-queries-on-the-same-table branch from ab829b1 to 9e0fce9 Compare December 15, 2019 00:19
@igdianov igdianov force-pushed the fix-GH-220-cannot-nest-multiple-queries-on-the-same-table branch from 9e0fce9 to 24891c1 Compare December 15, 2019 00:41
@igdianov igdianov merged commit 38bfb17 into master Dec 15, 2019
@igdianov igdianov deleted the fix-GH-220-cannot-nest-multiple-queries-on-the-same-table branch December 15, 2019 00:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot nest multiple queries on the same table: Invalid path: 'generatedAlias1.name'
2 participants