-
Notifications
You must be signed in to change notification settings - Fork 40
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
feat: Add explainable renderLimitNode & hardLimitNode attributes. #614
feat: Add explainable renderLimitNode & hardLimitNode attributes. #614
Conversation
Description: "Explain Query Request With Only Offset Specified.", | ||
|
||
Query: `query @explain { | ||
author(offset: 2) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did not know we could do this lol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was also first time finding out haha.
However not sure if that is the desired behavior?
Because if say you have a document in the collection, doing offset >= 1 (i.e. 1) will return no elements.
for example:
query {
author(offset: 1) {
name
}
}
Regardless of the behavior probably discussion for outside the scope of this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
definitely outside of this PR lol, but we seem to be lacking tests for this - will create an issue
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appreciate it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
7d4a224
to
6741047
Compare
Codecov Report
@@ Coverage Diff @@
## develop #614 +/- ##
===========================================
+ Coverage 56.84% 56.90% +0.05%
===========================================
Files 122 122
Lines 14595 14605 +10
===========================================
+ Hits 8297 8311 +14
+ Misses 5588 5584 -4
Partials 710 710
|
renderLimitNode
and hardLimitNode
attribute(s). …urcenetwork#614) - Resolves: sourcenetwork#561 - Description: Adds the `limit` and `offset` attributes for both `renderLimitNode` and `hardLimitNode` to be included in the returned explain graph response. - Request: ``` query @Explain { author(limit: 3, offset: 1) { numberOfArts: _count(articles: {}) articles(limit: 2) { name } } } ``` - Response: ``` { "data": [ { "explain": { "selectTopNode": { "hardLimitNode": { "limit": 3, "offset": 1, "countNode": { "sources": [ { "fieldName": "articles", "filter": null } ], "selectNode": { "filter": null, "typeIndexJoin": { "joinType": "typeJoinMany", "rootName": "author", "root": { "scanNode": { "collectionID": "3", "collectionName": "author", "filter": null, "spans": [ { "end": "/4", "start": "/3" } ] } }, "subTypeName": "articles", "subType": { "selectTopNode": { "renderLimitNode": { "limit": 2, "offset": 0, "selectNode": { "filter": null, "scanNode": { "collectionID": "1", "collectionName": "article", "filter": null, "spans": [ { "end": "/2", "start": "/1" } ] } } } } } } } } } } } } ] } ```
Relevant issue(s)
Resolves #561
Description
Adds the
limit
andoffset
attributes for bothrenderLimitNode
andhardLimitNode
to be included in the returned explain graph response.Example:
Request:
Response:
Tasks
How has this been tested?
Integration tests locally and only CI. Specifically
make test
.Specify the platform(s) on which this was tested: