Skip to content

Inline optional with where is not optional #233

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

Closed
molexx opened this issue Jan 13, 2020 · 9 comments · Fixed by #251
Closed

Inline optional with where is not optional #233

molexx opened this issue Jan 13, 2020 · 9 comments · Fixed by #251
Labels

Comments

@molexx
Copy link
Contributor

molexx commented Jan 13, 2020

I want to get all Humans, and include their favoriteDroid if their favoriteDroid's name begins with 'R'. I want Humans who don't have a favoriteDroid or who's favoriteDroid is C3PO to still be listed but have favoriteDroid: null.

But for this:

query {
  Humans {
    select {
      id
      name
      favoriteDroid(optional:true, where:{name:{LIKE:"R%"}}) {
        name
      }
    }
  }
}

the 'optional' appears to have no effect - most Humans are excluded.

If there is no where clause then it works as expected - Humans with no favoriteDroid are returned with a null favoriteDroid.

@igdianov
Copy link
Collaborator

@molexx It looks like a bug to me. :)

You can try use this query as workaround:

  Humans(where: {
    favoriteDroid: {
      OR:{
        id: {IS_NULL: true}
        name: {LIKE: "R"}
      }
    }
  }) {
    select {
      id
      name
      favoriteDroid {
        name
      }
    }
  }
{
  "data": {
    "Humans": {
      "select": [
        {
          "id": "1001",
          "name": "Darth Vader",
          "favoriteDroid": {
            "name": "R2-D2"
          }
        },
        {
          "id": "1002",
          "name": "Han Solo",
          "favoriteDroid": null
        },
        {
          "id": "1003",
          "name": "Leia Organa",
          "favoriteDroid": null
        },
        {
          "id": "1004",
          "name": "Wilhuff Tarkin",
          "favoriteDroid": null
        }
      ]
    }
  }
}

@igdianov igdianov added the bug label Jan 21, 2020
@molexx
Copy link
Contributor Author

molexx commented Jan 28, 2020

Thanks!

But the response doesn't include Luke, I expect him to be there but with "favoriteDroid": null because his favoriteDroid is C3PO.

@molexx
Copy link
Contributor Author

molexx commented Feb 1, 2020

I think the problem here is that in SQL an outer join will return no rows (as opposed to null) if the WHERE clause contains a predicate that doesn't match.
However, it is different in the outer join's ON clause where a null row is returned if the ON's predicates don't match.

So a possible fix for this is to move the predicates from the WHERE to each join's ON.

Unfortunately the JPA spec disallows adding ON predicates to fetch joins, they are only allowed on joins that aren't fetch, and all of this project's joins are fetch joins.

Is it possible to change that?
Where columns need to be fetched we would need to build up a fetch graph, would that be straightforward?

@igdianov
Copy link
Collaborator

igdianov commented Feb 3, 2020

@molexx Changing predicates to apply restrictions in JPA's JOIN ON for each join will not make a difference. The underlying SQL query will use either outer outer or inner join with restrictions on the join, i.e.

GraphQL JPQL Fetch Query:

select distinct human from Human as human 
  left join fetch human.favoriteDroid as generatedAlias0 
  where generatedAlias0.name like :param0  
  order by human.id asc

Is translated by Hibernate to:

    select
        human0_.id as id2_0_0_,
        droid1_.id as id2_0_1_,
        human0_.name as name3_0_0_,
        human0_.favorite_droid_id as favorite6_0_0_,
        human0_.gender_code_id as gender_c7_0_0_,
        human0_.homePlanet as homePlan4_0_0_,
        droid1_.name as name3_0_1_,
        droid1_.primary_function as primary_5_0_1_ 
    from
        Character human0_ 
    left outer join
        Character droid1_ 
            on human0_.favorite_droid_id=droid1_.id 
    where
        human0_.DTYPE='Human' 
        and (
            droid1_.name like ?
        ) 
    order by
        human0_.id asc

but it is also equivalent to the following query with left outer join where name restriction:

    select
        human0_.id as id2_0_0_,
        droid1_.id as id2_0_1_,
        human0_.name as name3_0_0_,
        human0_.favorite_droid_id as favorite6_0_0_,
        human0_.gender_code_id as gender_c7_0_0_,
        human0_.homePlanet as homePlan4_0_0_,
        droid1_.name as name3_0_1_,
        droid1_.primary_function as primary_5_0_1_ 
    from
        Character human0_ 
    left outer join
        Character droid1_ 
            on human0_.favorite_droid_id=droid1_.id 
            where droid1_.name like ?
    where
        human0_.DTYPE='Human' 
    order by
        human0_.id asc

In this plain query without name restriction all humans are returned with or without droid relation, which is optional by default:

{
 Humans
  {
    select {
      id
      name
      favoriteDroid {
        name
      }
    }
  }
}

{
  "data": {
    "Humans": {
      "select": [
        {
          "id": "1000",
          "name": "Luke Skywalker",
          "favoriteDroid": {
            "name": "C-3PO"
          }
        },
        {
          "id": "1001",
          "name": "Darth Vader",
          "favoriteDroid": {
            "name": "R2-D2"
          }
        },
        {
          "id": "1002",
          "name": "Han Solo",
          "favoriteDroid": null
        },
        {
          "id": "1003",
          "name": "Leia Organa",
          "favoriteDroid": null
        },
        {
          "id": "1004",
          "name": "Wilhuff Tarkin",
          "favoriteDroid": null
        }
      ]
    }
  }
}

Now, with name restriction, the SQL query applies it on the left outer join and correctly returns single Darth Vader:

{
  Humans(where: {
    favoriteDroid: {
      name: {LIKE: "R"}
    }
  }) {
    select {
      id
      name
      favoriteDroid {
        name
      }
    }
  }
}

{
  "data": {
    "Humans": {
      "select": [
        {
          "id": "1001",
          "name": "Darth Vader",
          "favoriteDroid": {
            "name": "R2-D2"
          }
        }
      ]
    }
  }
}

After relaxing name restriction to include humans without favorite droids, i.e.

{
  Humans(where: {
    favoriteDroid: {
      OR: {
      	name: {LIKE: "R"}
        id: {IS_NULL: true}
      }
    }
  }) 
  {
    select {
      id
      name
      favoriteDroid {
        name
      }
    }
  }
}

{
  "data": {
    "Humans": {
      "select": [
        {
          "id": "1001",
          "name": "Darth Vader",
          "favoriteDroid": {
            "name": "R2-D2"
          }
        },
        {
          "id": "1002",
          "name": "Han Solo",
          "favoriteDroid": null
        },
        {
          "id": "1003",
          "name": "Leia Organa",
          "favoriteDroid": null
        },
        {
          "id": "1004",
          "name": "Wilhuff Tarkin",
          "favoriteDroid": null
        }
      ]
    }
  }
}

So, I think there the query and results are consistent.

@molexx
Copy link
Contributor Author

molexx commented Feb 5, 2020

The last GraphQL response doesn't include Luke because his favoriteDroid is neither null nor begins with 'R'. But I'd like to be able to include Luke, with a null favoriteDroid.

In the original query I'm expecting all 5 Humans to be listed because the Humans aren't restricted and the 'name begins with R' predicate on favoriteDroid is marked as optional. It seems reasonable to expect that favoriteDroids with names that don't begin with R would not be returned but that the optional means that their parent (containing Human) still would be?

I tried the two SQL queries in the H2 console and they produce different results:

The first one, with droid1_.name like ? in the WHERE, returns only 1 row.
Adding or droid1_.name is null acts as in the last GraphQL above - 4 rows are returned excluding Luke.

But the second SQL, with droid1_.name like ? in the join's ON, returns 5 rows. All rows correctly have a null favoriteDroid except for Vader whos favoriteDroid begins with 'R'.

@igdianov
Copy link
Collaborator

igdianov commented Mar 30, 2020

@molexx I have been experimenting with fetch join on condition to try enabling collection filters using embedded where criteria expressions, i.e.

{
  where_condition: Authors(where: { books: {title: {LIKE: "War"}}}) {
    select {
      id
      name
      books {
        id
        title
      }
    }
  }
  
  on_condition: Authors {
    select {
      id
      name
      books(where: {title: {LIKE: "War"}}) {
        id
        title
      }
    }
  }
}

Each query produces different results:

{
  "data": {
    "where_condition": {
      "select": [
        {
          "id": 1,
          "name": "Leo Tolstoy",
          "books": [
            {
              "id": 2,
              "title": "War and Peace"
            }
          ]
        }
      ]
    },
    "on_condition": {
      "select": [
        {
          "id": 1,
          "name": "Leo Tolstoy",
          "books": [
            {
              "id": 2,
              "title": "War and Peace"
            }
          ]
        },
        {
          "id": 4,
          "name": "Anton Chekhov",
          "books": []
        },
        {
          "id": 8,
          "name": "Igor Dianov",
          "books": []
        }
      ]
    }
  }
}

The only caveat is I had to hack Hibernate to disable check for fetch join with on conditions:

https://github.com/introproventures/graphql-jpa-query/pull/251/files#diff-810c995d4bb529948f9490916b239daeR431-R436

The relevant change in the code is here:

https://github.com/introproventures/graphql-jpa-query/pull/251/files#diff-bd37bd3308e3bcfd0e3aab343aef2757R528-R538

@molexx
Copy link
Contributor Author

molexx commented Mar 31, 2020

Thanks for this @igdianov !

Is it okay to hack hibernate? Will we hit issues updating it and get stuck on a particular version?

Would an alternative be to not use fetch joins but instead build a fetch graph as it goes along?

@igdianov
Copy link
Collaborator

igdianov commented Apr 3, 2020

@molexx I know it is not a good idea to hack Hibernate. There only reason they have the restriction on fetch join condition is to follow JPA spec, so it is not a option.

The only viable option to implement collection fetch filter behavior is to create extra queries when resolving data for associated collections with inline where criteria, but it will have big impact on performance due to N+1 queries generated for each parent record. It might be possible to mitigate it with GraphQL batch loader, but I am not sure if this is going to work at all due to entity manager session binding to specific thread while batch loader execution will run on a different thread.

@igdianov
Copy link
Collaborator

igdianov commented Apr 6, 2020

@molexx Here is the fix for your issue: #251

Feel free to give a try and let me know if it works for you.

igdianov added a commit that referenced this issue Apr 7, 2020
… plural attributes (#251)

* fix: add ON condition on the right side of collection join fetch criteria

* fix: remove Hibernate HqlSqlWalker hack

* fix: add GraphQLJpaOneToManyDataFetcher for inline query filter

* fix: optimize OneToManyDataFetcher using query stream result

* fix: update name to GraphQLJpaOneToManyDataFetcher

* feat: add OneToMany batch DataLoader support

* fix: refactor GraphQLJpaOneToManyMappedBatchLoader

* feat: add GraphQLJpaToOneMappedBatchLoader  support

* fix: add batch query logging

* fix: add support orderBy sort for collections

* fix: unexpected NPE in getJPQLQueryString

* fix: add support for inline optional to one assiciations

* fix: add inline optional support for singular attributes

* fix: add support for inline collections filter with alias

* fix: remove hibernate-core dependency

* fix: polish data loader data fetchers
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
2 participants