-
Notifications
You must be signed in to change notification settings - Fork 86
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
Add support for conditional mutations #50
Conversation
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: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @srfrog)
example_mutation_txn_test.go, line 1 at r1 (raw file):
/*
Maybe move this file and the rest of the examples to a separate directory?
protos/api.proto, line 134 at r1 (raw file):
Value var_name = 8;
How is this field supposed to be used? I do not see it in the example.
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: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @martinmr)
example_mutation_txn_test.go, line 1 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
Maybe move this file and the rest of the examples to a separate directory?
Sure, but maybe not for this particular PR.
protos/api.proto, line 134 at r1 (raw file):
Previously, martinmr (Martin Martinez Rivera) wrote…
Value var_name = 8;
How is this field supposed to be used? I do not see it in the example.
VarName has the name of the query variable for this nquad. When assigning uids before mutation, we use the value from the variable to alias the subject. Similar to _:
but the inverse.
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: 0 of 3 files reviewed, 2 unresolved discussions (waiting on @martinmr)
Your examples are being run right now by the integration tests and failing. I would either disable the tests for now or revert them and re-add them once they can pass. It's fine if this PR only includes the API changes. |
…tation txn" This reverts commit e2e340d.
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: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @martinmr)
example_mutation_txn_test.go, line 1 at r1 (raw file):
Previously, srfrog (Gus) wrote…
Sure, but maybe not for this particular PR.
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: 0 of 2 files reviewed, 2 unresolved discussions (waiting on @martinmr)
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 3 of 3 files at r2.
Reviewable status:complete! all files reviewed, all discussions resolved
This PR adds initial API support for conditional mutations (
txn
block).New mutation signature:
Breaking changes:
cond_query
in Mutation gRPC message for sending conditional query.subject_var
in NQuad gRPC message to indicate variable name instead of subject uidobject_var
in NQuad gRPC message to indicate variable name instead of object idAn example of proper usage is also included.
This change is![Reviewable](https://camo.githubusercontent.com/1541c4039185914e83657d3683ec25920c672c6c5c7ab4240ee7bff601adec0b/68747470733a2f2f72657669657761626c652e696f2f7265766965775f627574746f6e2e737667)