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

feat: Add support to explain countNode attributes. #504

Merged
merged 2 commits into from
Jun 9, 2022

Conversation

shahzadlone
Copy link
Member

@shahzadlone shahzadlone commented Jun 7, 2022

RELEVANT ISSUE(S)

Resolves #478

DESCRIPTION

  • Adds ability to explain attributes of countNode.
  • Fixes a bug which was omitting the aggregate nodes.

Request:

query @explain {
	author {
		name
		numberOfBooks: _count(books: {})
	}
}

Response:

{
	"explain": {
		"selectTopNode": {
			"countNode": {
				"filter":         nil,
				"sourceProperty": "books",
				"selectNode": {
					"filter": nil,
					"typeIndexJoin": {
						"scanNode": {
							"collectionID":   "3",
							"collectionName": "author",
							"filter":         nil,
							"spans": []{
								{
									"start": "/3",
									"end":   "/4",
								}
							}
						}
					}
				}
			}
		}
	}
}

HOW HAS THIS BEEN TESTED?

Integration tests.

CHECKLIST:

  • I have commented the code, particularly in hard-to-understand areas.
  • I have made sure that the PR title adheres to the conventional commit style (subset of the ones we use can be found under: tools/configs/chglog/config.yml

ENVIRONMENT / OS THIS WAS TESTED ON?

Please specify which of the following was this tested on (remove or add your own):

  • Arch Linux

@shahzadlone shahzadlone added feature New feature or request area/query Related to the query component action/no-benchmark Skips the action that runs the benchmark. labels Jun 7, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.3 milestone Jun 7, 2022
@shahzadlone shahzadlone self-assigned this Jun 7, 2022
Copy link
Contributor

@AndrewSisley AndrewSisley left a comment

Choose a reason for hiding this comment

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

Looks good. Left an optional suggestion on selectTN.source

query/graphql/planner/select.go Outdated Show resolved Hide resolved
Copy link
Member

@jsimnz jsimnz left a comment

Choose a reason for hiding this comment

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

Straight forward, minor suggestion. LGTM

query/graphql/planner/count.go Outdated Show resolved Hide resolved
@shahzadlone shahzadlone force-pushed the lone/feat/explain-count-node branch from 74f9147 to 6ebd9a5 Compare June 9, 2022 01:05
@codecov
Copy link

codecov bot commented Jun 9, 2022

Codecov Report

Merging #504 (6ebd9a5) into develop (b8beabb) will increase coverage by 0.10%.
The diff coverage is 94.44%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop     #504      +/-   ##
===========================================
+ Coverage    54.10%   54.20%   +0.10%     
===========================================
  Files           97       97              
  Lines        13099    13109      +10     
===========================================
+ Hits          7087     7106      +19     
+ Misses        5337     5328       -9     
  Partials       675      675              
Impacted Files Coverage Δ
query/graphql/planner/planner.go 75.00% <66.66%> (ø)
query/graphql/planner/count.go 88.88% <100.00%> (+11.53%) ⬆️
query/graphql/planner/select.go 75.66% <100.00%> (ø)
query/graphql/planner/type_join.go 69.07% <100.00%> (ø)
query/graphql/planner/render.go 85.26% <0.00%> (+1.05%) ⬆️
query/graphql/planner/sort.go 75.67% <0.00%> (+1.35%) ⬆️
query/graphql/planner/limit.go 81.25% <0.00%> (+3.12%) ⬆️

@shahzadlone shahzadlone merged commit 954d22d into develop Jun 9, 2022
@shahzadlone shahzadlone deleted the lone/feat/explain-count-node branch June 9, 2022 01:21
shahzadlone added a commit to shahzadlone/defradb that referenced this pull request Feb 23, 2024
- RELEVANT ISSUE(S)

Resolves sourcenetwork#478 

- DESCRIPTION

(1) Adds ability to explain attributes of `countNode`.
(2) Fixes a bug which was omitting the aggregate nodes.

Request:
```
query @Explain {
	author {
		name
		numberOfBooks: _count(books: {})
	}
}
```

Response:
```
{
	"explain": {
		"selectTopNode": {
			"countNode": {
				"filter":         nil,
				"sourceProperty": "books",
				"selectNode": {
					"filter": nil,
					"typeIndexJoin": {
						"scanNode": {
							"collectionID":   "3",
							"collectionName": "author",
							"filter":         nil,
							"spans": []{
								{
									"start": "/3",
									"end":   "/4",
								}
							}
						}
					}
				}
			}
		}
	}
}
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/no-benchmark Skips the action that runs the benchmark. area/query Related to the query component feature New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Explain the attributes of countNode.
4 participants