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.

Sign up for GitHub

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

Jump to bottom

[Improvement] The semantics of delete operation for relational storage should be aligned with kv storage #3092

Closed
jerryshao opened this issue Apr 22, 2024 · 3 comments · Fixed by #3100
Closed

[Improvement] The semantics of delete operation for relational storage should be aligned with kv storage #3092

jerryshao opened this issue Apr 22, 2024 · 3 comments · Fixed by #3100
Assignees
@xloya
Labels
0.5.1 Release v0.5.1 0.6.0 Release v0.6.0 bug Something isn't working
Milestone
Gravitino June Re...

Comments

@jerryshao
Copy link
Contributor

jerryshao commented Apr 22, 2024 •
edited
Loading

What would you like to be improved?

Currently, for kv storage, the delete operation will return false if the key doesn't exist. Whereas, for relational store, it will throw an NoSuchEntityException, we should aligh relational store with kv store.

How should we improve?

No response
@jerryshao jerryshao added the improvement Improvements on everything label Apr 22, 2024
@jerryshao
Copy link
Contributor Author

@yuqi1129 @xloya would you please help to check this. The difference of two systems makes other modules hard to use.

@yuqi1129
Copy link
Contributor

Yeah, the JDBC backend will throw NoSuchEntityException if the entity to be deleted does not exist, so we will make it align with the KV backend and return false instead.

@xloya
Copy link
Contributor

xloya commented Apr 22, 2024

I will open a PR to hot fix this problem.

@xloya xloya mentioned this issue Apr 22, 2024
Merged
@jerryshao jerryshao added this to the Gravitino June Release milestone Apr 24, 2024
yuqi1129 pushed a commit that referenced this issue May 9, 2024
@xloya
[#3092] fix(relational-entity-store): Return false when deleting thro…
c740b13 
…w a `NoSuchEntityException` (#3100)

### What changes were proposed in this pull request?

When Relational Entity Store deleteing an entity and throwing a
NoSuchEntityException, return false finally. It's consistent with KV
Store.

### Why are the changes needed?

Fix: #3092 

### How was this patch tested?

Add some duplicate deleted uts.

---------

Co-authored-by: xiaojiebao <[email protected]>
github-actions bot pushed a commit that referenced this issue May 9, 2024
@xloya @web-flow
[#3092] fix(relational-entity-store): Return false when deleting thro…
098861e 
…w a `NoSuchEntityException` (#3100)

### What changes were proposed in this pull request?

When Relational Entity Store deleteing an entity and throwing a
NoSuchEntityException, return false finally. It's consistent with KV
Store.

### Why are the changes needed?

Fix: #3092 

### How was this patch tested?

Add some duplicate deleted uts.

---------

Co-authored-by: xiaojiebao <[email protected]>
@jerryshao jerryshao mentioned this issue May 9, 2024
Merged
@yuqi1129 yuqi1129 added 0.5.1 Release v0.5.1 0.6.0 Release v0.6.0 bug Something isn't working and removed improvement Improvements on everything labels May 9, 2024
yuqi1129 pushed a commit that referenced this issue May 9, 2024
@jerryshao @xloya
[#3092] fix(relational-entity-store): Return false when deleting thro…
f29b1d2 
…w a `NoSuchEntityException` (#3311)

### What changes were proposed in this pull request?

When Relational Entity Store deleteing an entity and throwing a
NoSuchEntityException, return false finally. It's consistent with KV
Store.

### Why are the changes needed?

Fix: #3092 

### How was this patch tested?

Add some duplicate deleted uts.

Co-authored-by: xloya <[email protected]>
Co-authored-by: xiaojiebao <[email protected]>
diqiu50 pushed a commit to diqiu50/gravitino that referenced this issue Jun 13, 2024
@xloya @diqiu50
[apache#3092] fix(relational-entity-store): Return false when deletin…
ee02221 
…g throw a `NoSuchEntityException` (apache#3100)

### What changes were proposed in this pull request?

When Relational Entity Store deleteing an entity and throwing a
NoSuchEntityException, return false finally. It's consistent with KV
Store.

### Why are the changes needed?

Fix: apache#3092 

### How was this patch tested?

Add some duplicate deleted uts.

---------

Co-authored-by: xiaojiebao <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@xloya xloya

Labels
0.5.1 Release v0.5.1 0.6.0 Release v0.6.0 bug Something isn't working
Projects
None yet
Milestone
Gravitino June Release
3 participants
@jerryshao @yuqi1129 @xloya