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

[UseSelection] fails on nullable relationship #1624

Closed
DorianGreen opened this issue Mar 30, 2020 · 6 comments
Closed

[UseSelection] fails on nullable relationship #1624

DorianGreen opened this issue Mar 30, 2020 · 6 comments
Assignees
Labels
🔍 investigate Indicates that an issue or pull request needs more information.

Comments

@DorianGreen
Copy link

Describe the bug
[UseSelection] compiles a query to an anonymous type, which breaks nullable optional relationships in EF Core.

To Reproduce

Model:

public class Foo
{
  public long Id { get; set; }
  public Bar? Bar { get; set; } //Bar is optional
}

public class Bar
{
  public long Id { get; set; }
  public string Name { get; set; } = default!; //Name is required
}

Query:

query
{
  foos
  {
     id,
     bar
     {
       id,
       name
     }
  }
}

Result:

"errors": [
    {
      "message": "Cannot return null for non-nullable field.",
      "locations": [
        {
          "line": 7,
          "column": 9
        }
      ],
      "path": [
        "foos",
        0,
        "bar",
        "name"
      ],
      "extensions": {
        "code": "EXEC_NON_NULL_VIOLATION"
      }
    }
  ]

Expected behavior

"data": {
    "workOrders": [
        {
          "id": 6,
          "requester": null
        },
        {
          "id": 5,
          "requester": {
            "id": 1,
            "name": "Sam"
          }
        },
        {
          "id": 4,
          "requester": {
            "id": 2,
            "name": "John"
          }
        }
      ]
    }

Problem
The query is compiled to the equivalent of:

context.Foos.Select(f => new
  {
      Id = f.Id,
      Bar = new { f.Bar.Id, f.Bar.Name }
  });

Because the selection is anonymous, EF has no idea that Bar should be null, and the result violates the GraphQL schema.

Solution
EF allows for null checks in the selection, which should mitigate the issue

context.Foos.Select(f => new
  {
      Id = f.Id,
      Bar = f.Bar == null ? null : new { f.Bar.Id, f.Bar.Name }
  });

HC Version: 11.0.0-preview.118
EF Core: 3.1.3

@michaelstaib michaelstaib added the 🔍 investigate Indicates that an issue or pull request needs more information. label Mar 30, 2020
@PascalSenn
Copy link
Member

Hi @DorianGreen
Thank you for opening this issue.
I am surprised that EF translates this like this.
We are not creating anonymous types though
It is

context.Foos.Select(f => new
  {
      Id = f.Id,
      Bar = new Bar() { Id = f.Bar.Id, Name = f.Bar.Name }
  });

I will look into this 👍

@DorianGreen
Copy link
Author

@PascalSenn , Thanks for the clarification.

The issue is, that EF will issue a left join from Foo to Bar, resulting in Id = 0 and Name = null.
In the current selection, a new Bar will be created even if it should be null.
The added null check will translate to a isnull(f.BarId) selection.

@DorianGreen
Copy link
Author

I have found a workaround for this issue.
I have updated Foo to:

public class Foo
{
  private Bar? _bar; 

  public long Id { get; set; }
  public Bar? Bar 
    {
       get => _bar == null ? null : _bar.Id == 0 ? null : _bar;
       set => _bar = value;
    }
}

This is really a hack around how EF Core handles nested selections and not a HC specific issue.

It would still be nice if the SelectionVisitor could insert a null check like the query I showed above, but it may not work with other IQueriable providers.

You can close this issue if it is not something you'd want to tackle in HC.

@DorianGreen
Copy link
Author

@PascalSenn I have ran into an issue with my workaround.
It will only work if the field selection contains bar.Id

@PascalSenn
Copy link
Member

@DorianGreen didn't have time to look into this properly the past few days. Will do this weekend 👍 I think this is a fix we can provide

@PascalSenn
Copy link
Member

Fixed in V11 and V10

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🔍 investigate Indicates that an issue or pull request needs more information.
Projects
None yet
Development

No branches or pull requests

3 participants