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

[Transactions] Are they really supported by the binary protocol? #4045

Closed
StarpTech opened this issue Apr 27, 2015 · 9 comments
Closed

[Transactions] Are they really supported by the binary protocol? #4045

StarpTech opened this issue Apr 27, 2015 · 9 comments
Assignees
Milestone

Comments

@StarpTech
Copy link

Example 1:

begin
LET t0 = select expand(in) from E where out.Mid = '553e97a7588219501f5af559' and @class = 'BroughtIntoLeft'
LET t1 = delete edge BroughtIntoLeft where out.Mid = '553e97a7588219501f5af559'
LET t2 = create vertex Node cluster DE_business content {"Email":"JohnDoe@local","Mid":"GF456FH6GHGRFGHGF34567","Points":0,"Position":"","DisplayName":"John Doe","Active":true}
LET t3 = create edge BroughtIntoLeft cluster DE_broughtIntoLeft from (select from Node where Mid = '123456789') to $t2
LET t4 = create edge BroughtIntoLeft cluster DE_broughtIntoLeft from $t2 to $t0
commit
return [$t0,$t1,$t2,$t3,$t4]

At command t3 I use a fictive ID 123456789 but it throws no exception or something like that.

Example 2

begin
LET t0 = select expand(in) from E where out.Mid = '553e96e0f6787a7c11c7b62f' and @class = 'BroughtIntoLeft'
LET t1 = delete edge BroughtIntoLeft where out.Mid = '553e96e0f6787a7c11c7b62f'
LET t2 = create vertex Node cluster DE_business content {"Email":"JohnDoe@local","Mid":"GF456FH6GHGRFGHGF34567","Points":0,"Position":"","DisplayName":"John Doe","Active":true}
LET t3 = create edge BroughtIntoLeft cluster DE_broughtIntoLeft from (select from Node where Mid = '553e96e0f6787a7c11c7b62f') to $t2
LET t4 = create edge BroughtIntoLeft cluster DE_broughtIntoLeft from $t2 to $t8
commit
return [$t0,$t1,$t2,$t3,$t4]

At command t4 I use a transaction result $t8 which isn't defined in the query. Like expected it throws an exception but my data aren't restored.

Example 3
Another simpler example: This transaction should throw an exception because the Node with the unique index property Email julia@local is already used.

begin
LET t0 = create vertex Node cluster DE_business content {"Email":"JohnDoe@local","Mid":"GF456FH6GHGRFGHGF34567","Points":0,"Position":"","DisplayName":"John Doe","Active":true}
LET t1 = create vertex Node cluster DE_business content {"Email":"julia@local","Mid":"FGZTFGZ456733333","Points":0,"Position":"","DisplayName":"Bernd Eck","Active":true}
LET t2 = create edge BroughtIntoLeft cluster DE_broughtIntoLeft from $t0 to $t1
commit
return [$t0,$t1,$t2]

My result:

[ { '@type': 'd',
    '@class': 'BroughtIntoLeft',
    '': null,
    '@rid': { cluster: 17, position: 7 } } ]

No data is written but I expect an found duplicated key exception.

Orientdb: 2.0.8
Nodejs Oriento driver

The transaction safety is very limited! Please improve this.

@StarpTech StarpTech changed the title Are transactions really supported by the binary protocol? [Transactions] Are they really supported by the binary protocol? Apr 30, 2015
@AlexFiverr
Copy link

👍 encountered all of these scenario in the REST API as well....

@tglman tglman self-assigned this May 5, 2015
@tglman tglman added this to the 2.1 GA milestone May 5, 2015
tglman added a commit that referenced this issue May 5, 2015
@tglman
Copy link
Member

tglman commented May 6, 2015

hi @StarpTech

I tried to reproduce this problems, and yes i could reproduce some of them, for 'example 2' a fix is merged in development, for 'example 3' we have already test case that cover that case and it works so i wasn't enable to reproduce it, for 'example 1' still doing some investigation.

thank you for reporting this :)

@StarpTech
Copy link
Author

Hi @tglman in your test for example 3 you don't create a unique dataset before you execute your batch script. Does it make a difference?

@tglman
Copy link
Member

tglman commented May 6, 2015

@StarpTech i was actually adding that test now :)

@StarpTech
Copy link
Author

@tglman awesome 👍 ^^

@StarpTech
Copy link
Author

Is it possible to make a hotfix? I think this is a big security risk when someone trusts in transactions.

tglman added a commit that referenced this issue May 7, 2015
tglman added a commit that referenced this issue May 7, 2015
…rations, issue #4045

Conflicts:
	graphdb/src/main/java/com/tinkerpop/blueprints/impls/orient/OrientElement.java
	graphdb/src/main/java/com/tinkerpop/blueprints/impls/orient/OrientVertex.java
@tglman
Copy link
Member

tglman commented May 7, 2015

ported the fixes also to 2.0.x branch, will be out with the 2.0.9

@StarpTech
Copy link
Author

👍 thanks.

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

No branches or pull requests

4 participants