Added variable substitution to FacetsFilter #4061
Conversation
harshil-goel
requested review from
manishrjain,
pawanrawal and
a team
as code owners
![pullrequest[bot]](https://avatars.githubusercontent.com/in/3434?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ A review job has been created and sent to the PullRequest network.
@harshil-goel you can click here to see the review status or cancel the code review job.
There was a problem hiding this comment.
Reviewed 3 of 3 files at r1.
Reviewable status:complete! all files reviewed, all discussions resolved (waiting on @manishrjain and @pawanrawal)
There was a problem hiding this comment.
I have left an inline comment regarding the backtick in one of the test. Please compare the commented version to the correct one to see the difference.
Reviewed with ❤️ by PullRequest
…ph-io/dgraph into harshil-goel/query-with-var
There was a problem hiding this comment.
Reviewed 2 of 3 files at r1, 1 of 1 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @harshil-goel and @manishrjain)
gql/parser_test.go, line 2040 at r2 (raw file):
q(func: has(works_in)) @cascade { name works_in @facets @facets(gt(since, $since)) {
And a couple of other filters as well here, taking in different values and see they are substituted as well.
query/query_facets_test.go, line 82 at r2 (raw file):
func TestFacetWithVar(t *testing.T) { triples := ` _:f <name> "user1" .
Do all this in a common place in populateClusterWithFacets please. Actually do you really need to have a new dataset for this? Can't you mint out a query with the existing data set?
There was a problem hiding this comment.
Reviewed 1 of 1 files at r3.
Reviewable status: all files reviewed, 5 unresolved discussions (waiting on @harshil-goel and @manishrjain)
gql/parser_test.go, line 2040 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
And a couple of other filters as well here, taking in different values and see they are substituted as well.
Please address this comment and then this PR is good to go.
query/query_facets_test.go, line 80 at r3 (raw file):
} func TestFacetWithVarLE(t *testing.T) {
Le
query/query_facets_test.go, line 101 at r3 (raw file):
} func TestFacetWithVarGT(t *testing.T) {
Gt
There was a problem hiding this comment.
Reviewable status: 2 of 3 files reviewed, 6 unresolved discussions (waiting on @balajijinnah, @manishrjain, @pawanrawal, and @pullrequest[bot])
gql/parser_test.go, line 2045 at r1 (raw file):
Previously, pullrequest[bot] wrote…
This backtick seems to be misplaced. Please move it to leftmost edge.
Done.
gql/parser_test.go, line 2040 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Please address this comment and then this PR is good to go.
Done.
query/query_facets_test.go, line 82 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Do all this in a common place in populateClusterWithFacets please. Actually do you really need to have a new dataset for this? Can't you mint out a query with the existing data set?
Done.
query/query_facets_test.go, line 80 at r3 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Le
Done.
query/query_facets_test.go, line 101 at r3 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Gt
Done.
There was a problem hiding this comment.
Reviewed 1 of 3 files at r1, 1 of 1 files at r2, 1 of 1 files at r4.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @balajijinnah, @pawanrawal, and @pullrequest[bot])
harshil-goel
requested review from
danielmai and
MichaelJCompton
as code owners
Fixes #2481
This change is