Skip to content

Conversation

@severussundar
Copy link
Contributor

Why make this change?

     {
        book_by_pk(id: 1){
             __typename
         }
     }
   

pk query bug

What is this change?

  • During the construction of SqlQueryStructure, if it is observed that there are no columns selected, then one of the primary keys is added so that the eventual database query constructed contains a valid column name in the SELECT statement.

How was this tested?

  • Integration Tests
  • Manual Tests

Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Waiting for few answers, perhaps the optimization of not hitting the db can be a followup PR

@aaronpowell
Copy link
Contributor

Waiting for few answers, perhaps the optimization of not hitting the db can be a followup PR

I would have expected that to be the better approach, since you don't actually need any data back from the DB, but I can see there being a problem when you've done a query like:

query {
  books {
    items {
      __typename
    }
  }
}

That should return a items array that has n __typename properties in it, where n is the count of records in the query (up to 100).

@severussundar severussundar requested a review from Aniruddh25 June 7, 2023 04:00
Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM after 1 optimization to generate expected output

Copy link
Collaborator

@Aniruddh25 Aniruddh25 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for addressing all comments.

Copy link
Contributor

@ayush3797 ayush3797 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@severussundar severussundar merged commit 7568c5b into main Jul 6, 2023
@severussundar severussundar deleted the dev/shyamsundarj/fix_typename_bug branch July 6, 2023 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working graphql

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Selection set without entity fields returns an error response for GraphQL query and mutation

5 participants