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

Fix(DQL): Fix Aggregate Functions on empty data #7176

Merged
merged 3 commits into from
Jan 5, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions graphql/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -645,11 +645,13 @@ func RunAll(t *testing.T) {
t.Run("query aggregate without filter", queryAggregateWithoutFilter)
t.Run("query aggregate with filter", queryAggregateWithFilter)
t.Run("query aggregate on empty data", queryAggregateOnEmptyData)
t.Run("query aggregate on empty scalar data", queryAggregateOnEmptyData2)
t.Run("query aggregate with alias", queryAggregateWithAlias)
t.Run("query aggregate with repeated fields", queryAggregateWithRepeatedFields)
t.Run("query aggregate at child level", queryAggregateAtChildLevel)
t.Run("query aggregate at child level with filter", queryAggregateAtChildLevelWithFilter)
t.Run("query aggregate at child level with empty data", queryAggregateAtChildLevelWithEmptyData)
t.Run("query aggregate at child level on empty scalar data", queryAggregateOnEmptyData3)
t.Run("query aggregate at child level with multiple alias", queryAggregateAtChildLevelWithMultipleAlias)
t.Run("query aggregate at child level with repeated fields", queryAggregateAtChildLevelWithRepeatedFields)
t.Run("query aggregate and other fields at child level", queryAggregateAndOtherFieldsAtChildLevel)
Expand Down
62 changes: 62 additions & 0 deletions graphql/e2e/common/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -2855,6 +2855,68 @@ func queryAggregateOnEmptyData(t *testing.T) {
string(gqlResponse.Data))
}

func queryAggregateOnEmptyData2(t *testing.T) {
queryPostParams := &GraphQLParams{
Query: `query {
aggregateState (filter: {xcode : { eq : "nsw" }} ) {
count
capitalMax
capitalMin
xcodeMin
xcodeMax
}
}`,
}

gqlResponse := queryPostParams.ExecuteAsPost(t, GraphqlURL)
RequireNoGQLErrors(t, gqlResponse)
testutil.CompareJSON(t,
`{
"aggregateState":
{
"capitalMax": null,
"capitalMin": null,
"xcodeMin": "nsw",
"xcodeMax": "nsw",
"count": 1
}
}`,
string(gqlResponse.Data))
}

func queryAggregateOnEmptyData3(t *testing.T) {
queryNumberOfStates := &GraphQLParams{
Query: `query
{
queryCountry(filter: { name: { eq: "India" } }) {
name
ag : statesAggregate {
count
nameMin
capitalMax
capitalMin
}
}
}`,
}
gqlResponse := queryNumberOfStates.ExecuteAsPost(t, GraphqlURL)
RequireNoGQLErrors(t, gqlResponse)
testutil.CompareJSON(t,
`
{
"queryCountry": [{
"name": "India",
"ag": {
"count" : 3,
"nameMin": "Gujarat",
"capitalMax": null,
"capitalMin": null
}
}]
}`,
string(gqlResponse.Data))
}

func queryAggregateWithoutFilter(t *testing.T) {
queryPostParams := &GraphQLParams{
Query: `query {
Expand Down
2 changes: 1 addition & 1 deletion query/outputnode.go
Original file line number Diff line number Diff line change
Expand Up @@ -914,7 +914,7 @@ func (sg *SubGraph) addAggregations(enc *encoder, fj fastJsonNode) error {
}
// the aggregation didn't happen, most likely was called with unset vars.
// See: query.go:fillVars
aggVal = types.Val{Tid: types.FloatID, Value: float64(0)}
// In this case we do nothing. The aggregate value in response will be returned as NULL.
}
if child.Params.Normalize && child.Params.Alias == "" {
continue
Expand Down
13 changes: 7 additions & 6 deletions query/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -990,11 +990,6 @@ func evalLevelAgg(
// The aggregated value doesn't really belong to a uid, we put it in UidToVal map
// corresponding to uid 0 to avoid defining another field in SubGraph.
vals := doneVars[needsVar].Vals
if len(vals) == 0 {
mp = make(map[uint64]types.Val)
mp[0] = types.Val{Tid: types.FloatID, Value: 0.0}
return mp, nil
}

ag := aggregator{
name: sg.SrcFunc.Name,
Expand Down Expand Up @@ -1724,7 +1719,13 @@ func (sg *SubGraph) fillVars(mp map[string]varValue) error {
if err := sg.replaceVarInFunc(); err != nil {
return err
}
lists = append(lists, sg.DestUIDs)

if len(sg.DestUIDs.GetUids()) > 0 {
// Don't add sg.DestUIDs in case its size is 0.
// This is to avoiding adding nil (empty element) to lists.
lists = append(lists, sg.DestUIDs)
}

sg.DestUIDs = algo.MergeSorted(lists)
return nil
}
Expand Down
62 changes: 61 additions & 1 deletion query/query1_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1473,7 +1473,7 @@ func TestAggregateRoot5(t *testing.T) {
}
`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"me":[{"sum(val(m))":0.000000}]}}`, js)
require.JSONEq(t, `{"data": {"me":[{"sum(val(m))":null}]}}`, js)
}

func TestAggregateRoot6(t *testing.T) {
Expand Down Expand Up @@ -1519,6 +1519,66 @@ func TestAggregateRootError(t *testing.T) {
require.Contains(t, err.Error(), "Only aggregated variables allowed within empty block.")
}

func TestAggregateEmptyData(t *testing.T) {

query := `
{
var(func: anyofterms(name, "Non-Existent-Data")) {
a as age
}

me() {
avg(val(a))
min(val(a))
max(val(a))
}
}
`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"me":[{"avg(val(a))":null},{"min(val(a))":null},{"max(val(a))":null}]}}`, js)
}

func TestCountEmptyData(t *testing.T) {

query := `
{
me(func: anyofterms(name, "Non-Existent-Data")) {
a: count(uid)
}
}
`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"me":[{"a":0}]}}`, js)
}

func TestCountEmptyData2(t *testing.T) {

query := `
{
a as var(func: eq(name, "Michonne"))
me(func: uid(a)) {
c: count(friend) @filter(eq(name, "non-existent"))
}
}
`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"me":[{"c":0}]}}`, js)
}

func TestCountEmptyData3(t *testing.T) {

query := `
{
a as var(func: eq(name, "Michonne"))
me(func: uid(a)) {
c: count(friend2)
}
}
`
js := processQueryNoErr(t, query)
require.JSONEq(t, `{"data": {"me":[]}}`, js)
}

func TestAggregateEmpty1(t *testing.T) {
query := `
{
Expand Down