nuwave / lighthouse

A framework for serving GraphQL from Laravel
https://lighthouse-php.com
MIT License
3.36k stars 437 forks source link

Complex where queries: Integrity constraint violation: 1052 Column "XXX" in where clause is ambiguous #1470

Open Jofairden opened 4 years ago

Jofairden commented 4 years ago

Describe the bug

Querying a certain field on a relation of the resulting type of the query and applying a complex where query to filter the relation causes the SQL error: SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'ID' in where clause is ambiguous

Expected behavior/Solution

I feel like selections in the query need to be prefixed, or addSelect should be used (if it isn't already)

Steps to reproduce

  1. Query some data and request a field on a relation of the data
  2. Have a hasX complex where filter to filter the queried field, e.g. ID or name etc.

Output/Logs

Example error: (see example code below)

``` "debugMessage": "SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'ID' in where clause is ambiguous (SQL: select count(*) as aggregate from `teams` where exists (select * from `players` inner join `player_team` on `player_team`.`player_id` = `players`.`id` where `teams`.`id` = `player_team`.`team_id` and (`ID` = 120)))", "message": "Internal server error", "extensions": { "category": "internal" }, ```

**Lighthouse Version** `"nuwave/lighthouse": "^4.12",` # Example code: Query: (Requesting teams of players, request players of the teams (paginated) and filter the players relation for an ID) ``` query { teams( hasPlayers: { column: ID value:120 } ) { data { id players { data { id } } } } } ``` Example GraphQL query file: ``` extend type Query @guard(with: ["api"]) { teams( hasPlayers: _ @whereHasConditions(columnsEnum: PlayerColumn) ): [Team!] @paginate } ``` Team type: ``` type Team { "The ID of this team" id: ID! "Players of this team" players: [Player!] @hasMany(type: "paginator") } ``` Player type: ``` type Player { "The ID of this player" id: ID! } ``` Playercolumn example: ``` enum PlayerColumn { ID } ```
spawnia commented 4 years ago

Please add a PR with a failing test case in https://github.com/nuwave/lighthouse/blob/master/tests/Integration/WhereConditions/WhereHasConditionsDirectiveTest.php in order to investigate this further.

By the way (not invalidating your issue), but have you considered using the following query instead:

{
  player(id: 120) {
    teams {
      players { ... }
    }
  }
}

It is generally a good idea to take advantage of the fact that a GraphQL query does indeed form a graph of data and traverse it as such.

Jofairden commented 4 years ago

Please add a PR with a failing test case in https://github.com/nuwave/lighthouse/blob/master/tests/Integration/WhereConditions/WhereHasConditionsDirectiveTest.php in order to investigate this further.

By the way (not invalidating your issue), but have you considered using the following query instead:

{
  player(id: 120) {
    teams {
      players { ... }
    }
  }
}

It is generally a good idea to take advantage of the fact that a GraphQL query does indeed form a graph of data and traverse it as such.

I know you can reverse/change the queries, maybe my example was bad by just showing one ID, but this happens in many cases, for example filtering by a collection of IDs using the IN operator. I'll see if I can add a PR with a test.

Jofairden commented 4 years ago

Come to think of it, I think this issue is relevant: https://github.com/nuwave/lighthouse/issues/1356 If I change my SQL query so one of the IDs is named, e.g. a.id, the issue goes away (At least in my SQL editor)

spawnia commented 4 years ago

Can you check https://github.com/nuwave/lighthouse/pull/1253 for inspiration?

daniel-de-wit commented 4 years ago

I'm new to this project and not sure if I followed the rules correctly.. But I added a failing test ☝️

spawnia commented 4 years ago

Another related issue: https://github.com/nuwave/lighthouse/issues/1214

I feel like all of those could be solved upstream by Laravel itself. If not, we have to think long and hard about how we can produce a robust solution.

daniel-de-wit commented 4 years ago

@spawnia Would you mind taking a look at my updated PR? https://github.com/nuwave/lighthouse/pull/1530

It solves this particular issue without breaking the tests