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

GraphQL function calls can overwrite eachother #1682

Open
tub opened this issue Mar 8, 2022 · 2 comments
Open

GraphQL function calls can overwrite eachother #1682

tub opened this issue Mar 8, 2022 · 2 comments
Labels
GraphQL CQL-first issues related to the GraphQL API

Comments

@tub
Copy link
Contributor

tub commented Mar 8, 2022

If you call the same function more than once in the same (CQL-first) graphql query, one result "wins" and every result for that function call is identical.
This appears to be because the result is a map keyed on the field-name, and since the field-name is the function name, you can effectively only have one result per function.

I've updated the integration tests to cover this case here - tub@e460bdf

It seems like it would need a fairly sizable refactor in order to change how the response is returned to graphql-java, but if anyone has any pointers on how it could be implemented I might be able to find time to do it & submit a patch.

@olim7t
Copy link
Collaborator

olim7t commented Mar 10, 2022

This is indeed a bug, and I agree with you that the fix is probably not trivial.

The problem with returning generic maps (as we do for rows) is that the GraphQL runtime maps the results using the actual field names, not their aliases in the GraphQL query.

I ran into a similar issue in the schema service. Queries like this one cannot be implemented with maps:

query { keyspace(name:"library") { 
  books: table(name: "books") { columns { name } }
  authors: table(name: "authors") { columns { name } }
}

Because GraphQL looks for a table key in the map for both fields. Even if you manage to put two entries books and authors in the map, they are ignored.

The solution at the time was to turn the map into a DTO, and resolve the field with a method: KeyspaceDto.getTable(DataFetchingEnvironment). Then that method can use its environment argument to figure out more about its invocation context.

This is the same in your example: even though sumPrice and sumSellPrice are different aliases, they designate the same field _decimal_function, hence the collision if we use a map. The problem was hidden until we added aggregations, because regular columns have naturally distinct names.
I think we need to change how we represent rows in response, but it's not as simple as just switching to a DTO, because our field names are still dynamic. I'm going to try a few things locally and I'll report back.

Related:

@olim7t olim7t added the GraphQL CQL-first issues related to the GraphQL API label Mar 10, 2022
@olim7t
Copy link
Collaborator

olim7t commented Mar 11, 2022

I've posted a proof of concept here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GraphQL CQL-first issues related to the GraphQL API
Projects
None yet
Development

No branches or pull requests

2 participants