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(DQL): @groupby on scalar fields and count duplicate #7746

Merged
merged 12 commits into from
Apr 30, 2021

Conversation

minhaj-shakeel
Copy link
Contributor

@minhaj-shakeel minhaj-shakeel commented Apr 23, 2021

This PR extends support for var inside the @groupby query on a scalar predicate at root.
for example the given query now doesn't result to error:-

var(func: ... ) @groupby(age) {
  c as count(uid)
}

Now the uid variable c contains map of uid to count of uids with the same age.
Suppose for the data :-

        "uid": "0x1",
        "age": 38
      },
      {
        "uid": "0x17",
        "age": 15
      },
      {
        "uid": "0x18",
        "age": 15
      },
      {
        "uid": "0x19",
        "age": 17
      }

The following groupby query:-

var(func: uid(1, 23, 24, 25)) @groupby(age) {
      c as count(uid)
}
 me(func: uid(c)) {
      uid
      val(c)
}

returns the result:-

"data": {
    "me": [
      {
        "uid": "0x1",
        "val(c)": 1
      },
      {
        "uid": "0x17",
        "val(c)": 2
      },
      {
        "uid": "0x18",
        "val(c)": 2
      },
      {
        "uid": "0x19",
        "val(c)": 1
      }
    ]
  }

The behaviour will be the same if there are multiple predicates and some of them are uid predicates.
However, it is only supported at root currently. Hence the given query will still return error:-

var(func: .....) {
   friend @groupby(age) {
       a as count(uid)
   }
}

This change is Reviewable

Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @minhaj-shakeel)


query/groupby.go, line 320 at r1 (raw file):

			// Currently vars are supported only at the root.
			// for eg, The following query will result into error:-

Add a detailed comment about why is this behavior not allowed.


query/query0_test.go, line 1480 at r1 (raw file):

			c as count(uid)
		}
		me(func: uid(c)) {

Add an example of doing a filter based on val(c) as well. Also check with Anand and see if we are satisfying the requirements of the customers that the request came from.

Copy link
Contributor Author

@minhaj-shakeel minhaj-shakeel left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @pawanrawal)


query/groupby.go, line 320 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Add a detailed comment about why is this behavior not allowed.

Done.


query/query0_test.go, line 1480 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Add an example of doing a filter based on val(c) as well. Also check with Anand and see if we are satisfying the requirements of the customers that the request came from.

Done.

Copy link
Contributor

@pawanrawal pawanrawal left a comment

Choose a reason for hiding this comment

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

Should be creating a doc ticket for this as well.

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @minhaj-shakeel)


query/groupby.go, line 388 at r2 (raw file):

				// uid -> count of duplicates. for eg if there are two predicates(u & v) and
				// the groupby uids for (u1, v1) pair are (uid1, uid2, uid3) then the variable
				// stores (uid1, 3), (uid2, 3) & (uid2, 2) map.

Maybe a bigger example with something like name and friend would help here.

Copy link
Contributor Author

@minhaj-shakeel minhaj-shakeel left a comment

Choose a reason for hiding this comment

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

Created the ticket.

Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @pawanrawal)


query/groupby.go, line 388 at r2 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Maybe a bigger example with something like name and friend would help here.

Adding it.

@benwoodward
Copy link

benwoodward commented Oct 4, 2022

can this be put into the next release please?

I have a query that doesn't work (using 21.03):

  queryDistinctLexicalEntryHeadwords(lexicon: String!, first: Int, offset: Int): GroupedPropertyMap
    @custom(
      dql: """
      query q($lexicon: string, $first: int, $offset: int = 0) {
        var(func: type(Lexicon), first: $first, offset: $offset) @filter(eq(Lexicon.language, $lexicon)) {
          Lexicon.lexicalEntries {
            LE as uid
          }
        }
        queryDistinctLexicalEntryHeadwords(func: uid(LE)) @groupby(headword: LexicalEntry.headword, count) {
          count(uid)
        }
      }
      """
    )

  # this is not possible in 21.03, but fix has been merged into master:
  # https://github.com/dgraph-io/dgraph/pull/7746
  #
  # queryDistinctLexicalEntryHeadwordsMetadata(lexicon: String!, first: Int, offset: Int): Metadata
  #   @custom(
  #     dql: """
  #     query q($lexicon: string, $first: int, $offset: int) {
  #       var(func: type(Lexicon), first: $first, offset: $offset) @filter(eq(Lexicon.language, $lexicon)) {
  #         Lexicon.lexicalEntries {
  #           LE as count(uid)
  #         }
  #       }
  #       queryDistinctLexicalEntryHeadwords(func: uid(LE)) @groupby(headword: LexicalEntry.headword, count) {
  #         totalCount: sum(val(LE))
  #       }
  #     }
  #     """
  #   )

the commented query gives me the following error:

Vars can be assigned only when grouped by UID attribute while executing this query

relevant: https://discuss.dgraph.io/t/vars-can-be-assigned-only-when-grouped-by-uid-attribute/16996/2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants