Skip to content

docs(Query): Update docs to note that only typed data can be deleted using S * *(Docs) #6657

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

Merged
merged 9 commits into from
Oct 13, 2020

Conversation

aaroncarey
Copy link
Contributor

@aaroncarey aaroncarey commented Oct 5, 2020

Customers are confused by what happens when they try to delete data that they haven't typed. This fix adds a note about this requirement and links to information on how customers can type their data.

Fixes GRAPHQL-669


This change is Reviewable

Docs Preview: Dgraph Preview

@CLAassistant
Copy link

CLAassistant commented Oct 5, 2020

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the area/documentation Documentation related issues. label Oct 5, 2020
…eleting data using a wildcard search

Per Michael Compton, the limitation is not as severe as I thought with the initial draft.
@aaroncarey aaroncarey changed the title docs(Query): Update docs to note that only typed data can be deleted (Docs) docs(Query): Update docs to note that only typed data can be deleted using * (Docs) Oct 5, 2020
@aaroncarey
Copy link
Contributor Author

Here's the link directly to the page with the change on the preview site:

https://dgraph-76b6db82fc-99471.surge.sh/mutations/delete/

Copy link
Contributor

@MichaelJCompton MichaelJCompton 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, 3 unresolved discussions (waiting on @aaroncarey, @bucanero, @danielmai, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)


wiki/content/mutations/delete.md, line 43 at r2 (raw file):

edges on that node and their corresponding definitions in the schema). 

**Note**: If the type information

Should this use this kind of note?

{{% notice "note" %}}


wiki/content/mutations/delete.md, line 44 at r2 (raw file):

**Note**: If the type information
for the node `S` is missing then the delete operation will not work. For more information on how to set the type

Let's explicitly call out that it's S * * mutations that need types (the S P * ones will still work).

So maybe we should make this note like "The S * * mutation pattern ..."


wiki/content/mutations/delete.md.sb-e9c19abf-QiH8oo, line 1 at r2 (raw file):

+++

What's this file?

@aaroncarey aaroncarey self-assigned this Oct 6, 2020
Deleted a file generated by my text editor. Thank you to Michael for catching this!
@aaroncarey
Copy link
Contributor Author

Let's explicitly call out that it's S * * mutations that need types (the S P * ones will still work).

So maybe we should make this note like "The S * * mutation pattern ..."

This is a great clarification, I will work it into the docs! Stay tuned for a new commit.

@aaroncarey
Copy link
Contributor Author

Reviewable status: 0 of 2 files reviewed, 3 unresolved discussions (waiting on @aaroncarey, @bucanero, @danielmai, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)

wiki/content/mutations/delete.md, line 43 at r2 (raw file):

edges on that node and their corresponding definitions in the schema). 

**Note**: If the type information

Should this use this kind of note?

{{% notice "note" %}}

wiki/content/mutations/delete.md, line 44 at r2 (raw file):

**Note**: If the type information
for the node `S` is missing then the delete operation will not work. For more information on how to set the type

Let's explicitly call out that it's S * * mutations that need types (the S P * ones will still work).

So maybe we should make this note like "The S * * mutation pattern ..."

wiki/content/mutations/delete.md.sb-e9c19abf-QiH8oo, line 1 at r2 (raw file):

+++

What's this file?

Garbage from my text editor, now removed. Thanks!

Copy link
Contributor

@MichelDiz MichelDiz 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 1 files reviewed, 7 unresolved discussions (waiting on @aaroncarey, @bucanero, @danielmai, @MichaelJCompton, @pawanrawal, and @vvbalaji-dgraph)


wiki/content/mutations/delete.md, line 9 at r3 (raw file):

removes predicates

I think triples suits better. As fundamentally the delete operation is removing 3 information from the DB.


wiki/content/mutations/delete.md, line 43 at r3 (raw file):

edges and any indexing for the removed data. The predicates to delete are
derived from the type information for that node (the value of the `dgraph.type`
edges on that node and their corresponding definitions in the schema). If the type information

why remove this?


wiki/content/mutations/delete.md, line 67 at r3 (raw file):

'wildcard delete'

“Star notation” also is fine.


wiki/content/mutations/delete.md.sb-e9c19abf-QiH8oo, line 1 at r2 (raw file):

Previously, MichaelJCompton (Michael Compton) wrote…

What's this file?

Yeah, this file is a bit odd. Feels like a copy of the delete.md.


wiki/content/mutations/delete.md.sb-e9c19abf-QiH8oo, line 11 at r3 (raw file):

A delete mutation, signified with the `delete` keyword, removes triples from the store. Note that you can only delete nodes if your data is typed using the `dgraph.type`predicate. For more imformation on setting the type of a node, see [Setting the type of a node](https://dgraph.io/docs/query-language/type-system/#setting-the-type-of-a-node).

For example, if the store contained the following:

add a space between the text and the code block.

Copy link
Contributor

@bucanero bucanero 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 1 files reviewed, 8 unresolved discussions (waiting on @aaroncarey, @bucanero, @danielmai, @MichaelJCompton, @MichelDiz, @pawanrawal, and @vvbalaji-dgraph)


wiki/content/mutations/delete.md, line 67 at r3 (raw file):

Previously, MichelDiz (Michel Diz) wrote…
'wildcard delete'

“Star notation” also is fine.

if you keep "wildcard delete", I'd suggest linking it to the previous heading. Something like:

Using a [wildcard delete](#wildcard-delete) star notation mentioned...

wiki/content/mutations/delete.md, line 9 at r4 (raw file):

signified

Just a suggestion, I'd go with "identified" instead of signified

Update to address remaining feedback on this PR.
Copy link
Contributor Author

@aaroncarey aaroncarey 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 1 files reviewed, 8 unresolved discussions (waiting on @bucanero, @danielmai, @MichaelJCompton, @MichelDiz, @pawanrawal, and @vvbalaji-dgraph)


wiki/content/mutations/delete.md, line 43 at r2 (raw file):

Previously, MichaelJCompton (Michael Compton) wrote…

Should this use this kind of note?

{{% notice "note" %}}

I moved this into a note, thanks!


wiki/content/mutations/delete.md, line 44 at r2 (raw file):

Previously, MichaelJCompton (Michael Compton) wrote…

Let's explicitly call out that it's S * * mutations that need types (the S P * ones will still work).

So maybe we should make this note like "The S * * mutation pattern ..."

Done. Lines 45-47 call this out.


wiki/content/mutations/delete.md, line 9 at r3 (raw file):

predicates

Thanks for raising this, I definitely want to use the right word, here. I'd like to settle on the most widely-understood term that works for our users. I don't see the term "triple" used this way in the latest GraphQL spec (http://spec.graphql.org/June2018/#sec-Language.Fields). That spec makes me wonder if "Block String" or "Object" isn't really the term we are looking for.

+++

After looking at this more closely, I'd like to avoid using the term "triple" here, as "predicate" or "field" seem to be the most widely-understood terms for the data contained within a node.


wiki/content/mutations/delete.md, line 43 at r3 (raw file):

Previously, MichelDiz (Michel Diz) wrote…

why remove this?

I reworked this material, nothing should be missing at this point


wiki/content/mutations/delete.md, line 67 at r3 (raw file):

Previously, bucanero (Damián Parrino) wrote…

if you keep "wildcard delete", I'd suggest linking it to the previous heading. Something like:

Using a [wildcard delete](#wildcard-delete) star notation mentioned...

Done.


wiki/content/mutations/delete.md, line 9 at r4 (raw file):

Previously, bucanero (Damián Parrino) wrote…
signified

Just a suggestion, I'd go with "identified" instead of signified

OK, done


wiki/content/mutations/delete.md.sb-e9c19abf-QiH8oo, line 1 at r2 (raw file):

Previously, MichelDiz (Michel Diz) wrote…

Yeah, this file is a bit odd. Feels like a copy of the delete.md.

Done.


wiki/content/mutations/delete.md.sb-e9c19abf-QiH8oo, line 11 at r3 (raw file):

Previously, MichelDiz (Michel Diz) wrote…

add a space between the text and the code block.

Done.

@aaroncarey
Copy link
Contributor Author

https://dgraph-dc27d1337c-100924.surge.sh/mutations/delete/ Shows the current state of this document.

@aaroncarey aaroncarey requested a review from bucanero October 12, 2020 20:34
Copy link
Contributor

@MichelDiz MichelDiz 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 1 files reviewed, 7 unresolved discussions (waiting on @aaroncarey, @bucanero, @danielmai, @MichaelJCompton, @MichelDiz, @pawanrawal, and @vvbalaji-dgraph)


wiki/content/mutations/delete.md, line 9 at r3 (raw file):

Previously, aaroncarey (Aaron Carey) wrote…
predicates

Thanks for raising this, I definitely want to use the right word, here. I'd like to settle on the most widely-understood term that works for our users. I don't see the term "triple" used this way in the latest GraphQL spec (http://spec.graphql.org/June2018/#sec-Language.Fields). That spec makes me wonder if "Block String" or "Object" isn't really the term we are looking for.

+++

After looking at this more closely, I'd like to avoid using the term "triple" here, as "predicate" or "field" seem to be the most widely-understood terms for the data contained within a node.

I think you didn't understand. This docs isn't GraphQL related. DQL is a totally different thing, and we use terms that define what Dgraph is. As you can see https://dgraph.io/docs/mutations/triples/ https://dgraph.io/docs/deploy/dgraph-administration/#deleting-database triples is a common thing in Dgraph. There are more references. This term comes from RDF standard. And it is a fundamental thing in Dgraph.

When you delete something, you are fundamentally deleting 3 information on the DB. It is like KV, when you delete a value from a KV, you are the Key and the Value. Another example. When you delete a whole node in Dgraph. You are actually deleting all triples, not just the values.

"Predicate" is just the "name of the key". In Dgraph when you say you are deleting a predicate, it is understood that you did not delete other information.

Check this conversation a bit https://dgraphlabs.slack.com/archives/CSEFVTFQU/p1594243043106000?thread_ts=1594222913.084400&cid=CSEFVTFQU

Changed the reference to "triples" back to the correct terminology, updated related statements.
@aaroncarey aaroncarey changed the title docs(Query): Update docs to note that only typed data can be deleted using * (Docs) docs(Query): Update docs to note that only typed data can be deleted using S * *(Docs) Oct 12, 2020
Copy link
Contributor Author

@aaroncarey aaroncarey 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 1 files reviewed, 7 unresolved discussions (waiting on @bucanero, @danielmai, @MichaelJCompton, @MichelDiz, @pawanrawal, and @vvbalaji-dgraph)


wiki/content/mutations/delete.md, line 9 at r3 (raw file):

Previously, MichelDiz (Michel Diz) wrote…

I think you didn't understand. This docs isn't GraphQL related. DQL is a totally different thing, and we use terms that define what Dgraph is. As you can see https://dgraph.io/docs/mutations/triples/ https://dgraph.io/docs/deploy/dgraph-administration/#deleting-database triples is a common thing in Dgraph. There are more references. This term comes from RDF standard. And it is a fundamental thing in Dgraph.

When you delete something, you are fundamentally deleting 3 information on the DB. It is like KV, when you delete a value from a KV, you are the Key and the Value. Another example. When you delete a whole node in Dgraph. You are actually deleting all triples, not just the values.

"Predicate" is just the "name of the key". In Dgraph when you say you are deleting a predicate, it is understood that you did not delete other information.

Check this conversation a bit https://dgraphlabs.slack.com/archives/CSEFVTFQU/p1594243043106000?thread_ts=1594222913.084400&cid=CSEFVTFQU

Thanks for the clarification! I reviewed the links you provided and I have updated references back to "triple" where appropriate. Please take another look.

Copy link
Contributor

@MichelDiz MichelDiz left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 1 of 1 files at r4, 1 of 1 files at r6.
Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @bucanero, @danielmai, @MichaelJCompton, @MichelDiz, @pawanrawal, and @vvbalaji-dgraph)

Copy link
Contributor Author

@aaroncarey aaroncarey left a comment

Choose a reason for hiding this comment

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

I think I have all of your feedback addressed, let me know if I missed anything :)

Copy link
Contributor

@bucanero bucanero left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: all files reviewed, 6 unresolved discussions (waiting on @bucanero, @danielmai, @MichaelJCompton, @MichelDiz, @pawanrawal, and @vvbalaji-dgraph)

Copy link
Contributor Author

@aaroncarey aaroncarey left a comment

Choose a reason for hiding this comment

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

Doc updated per your feedback!

Copy link
Contributor Author

@aaroncarey aaroncarey left a comment

Choose a reason for hiding this comment

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

Extra file deleted, now using a better markdown editor 👍

Copy link
Contributor Author

@aaroncarey aaroncarey left a comment

Choose a reason for hiding this comment

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

Feedback addressed, thanks!

Copy link
Contributor Author

@aaroncarey aaroncarey left a comment

Choose a reason for hiding this comment

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

Requested fixes implemented

Copy link

@codelingo codelingo bot left a comment

Choose a reason for hiding this comment

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

LGTM. 32 rules errored during the review.

Copy link

@codelingo codelingo bot left a comment

Choose a reason for hiding this comment

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

LGTM. 32 rules errored during the review.

Updated the text of the note on line 47 per feedback from Michael C.
Copy link
Contributor

@MichaelJCompton MichaelJCompton left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewable status: 0 of 1 files reviewed, 6 unresolved discussions (waiting on @bucanero, @danielmai, @MichaelJCompton, @MichelDiz, @pawanrawal, and @vvbalaji-dgraph)

@aaroncarey aaroncarey merged commit e2fc874 into master Oct 13, 2020
@aaroncarey aaroncarey deleted the delete-by-type branch October 13, 2020 03:36
Copy link

@codelingo codelingo bot left a comment

Choose a reason for hiding this comment

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

LGTM. 32 rules errored during the review.

Copy link

@codelingo codelingo bot left a comment

Choose a reason for hiding this comment

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

LGTM. 32 rules errored during the review.

aaroncarey added a commit that referenced this pull request Oct 23, 2020
Cherry-pick of fixes to this file from PR #6657
danielmai pushed a commit that referenced this pull request Oct 24, 2020
…an be deleted using `S * *` (#6798)

Cherry-pick of fixes to this file from PR #6657

Fixes GRAPHQL-751
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Documentation related issues.
Development

Successfully merging this pull request may close these issues.

5 participants