-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Allow deletion of specific language. #3242
Conversation
This change introduces the syntax "<s> <p@lang> * ." to allow users to delete a specific language tagged value without having to specify the current value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 4 of 4 files at r1.
Reviewable status: complete! all files reviewed, all discussions resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all files reviewed, all discussions resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @manishrjain and @martinmr)
a discussion (no related file):
Can we extend this to JSON? e.g:
{
"delete": [
{
"uid": "0x12345",
"name@es": null
}
]
}
equivalent to:
<0x12345> <name@es> * .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @manishrjain and @MichelDiz)
a discussion (no related file):
Previously, MichelDiz (Michel Conrado) wrote…
Can we extend this to JSON? e.g:
{ "delete": [ { "uid": "0x12345", "name@es": null } ] }
equivalent to:
<0x12345> <name@es> * .
it should already work for JSON if the two representations are identical (produce the same DirectedEdge object when parsed). I'll add a test to the json parser to verify this is the case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @manishrjain and @MichelDiz)
a discussion (no related file):
Previously, martinmr (Martin Martinez Rivera) wrote…
it should already work for JSON if the two representations are identical (produce the same DirectedEdge object when parsed). I'll add a test to the json parser to verify this is the case.
Added tests but this is the right syntax.
{
"delete": [
{
"uid": "0x12345",
"name@es": ""
}
]
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @manishrjain and @MichelDiz)
a discussion (no related file):
Previously, martinmr (Martin Martinez Rivera) wrote…
Added tests but this is the right syntax.
{ "delete": [ { "uid": "0x12345", "name@es": "" } ] }
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @manishrjain and @martinmr)
a discussion (no related file):
Previously, martinmr (Martin Martinez Rivera) wrote…
Done.
I see, the empty string works indeed. But Docs says that "rating": null
is a S P *
operation syntax. https://docs.dgraph.io/mutations/#deleting-edges - If empty strings are to be considered, for me it's okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 4 of 5 files reviewed, all discussions resolved (waiting on @manishrjain)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 4 of 5 files reviewed, all discussions resolved (waiting on @manishrjain)
a discussion (no related file):
Previously, MichelDiz (Michel Conrado) wrote…
I see, the empty string works indeed. But Docs says that
"rating": null
is aS P *
operation syntax. https://docs.dgraph.io/mutations/#deleting-edges - If empty strings are to be considered, for me it's okay.
This seems like a bug. The language tag is not being parsed properly. I'll revert these test changes and open a new PR to fix the bug. But otherwise, JSON support should work out of the box.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And how would this deletion work in JSON?
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @manishrjain)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to submit #3243 to make this work but it should work in the following way:
{
"uid":"0xa",
"name@en":null
}
Reviewable status: complete! all files reviewed, all discussions resolved
This change introduces the syntax "<s> <p@lang> * ." to allow users to delete a specific language tagged value without having to specify the current value.
This change introduces the syntax
"<s> <p@lang> * ."
to allow users todelete a specific language tagged value without having to specify the
current value.
Closes #3237
This change is