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

[#3092] fix(relational-entity-store): Return false when deleting throw a NoSuchEntityException #3100

Merged
merged 5 commits into from
May 9, 2024
Merged

[#3092] fix(relational-entity-store): Return false when deleting throw a NoSuchEntityException #3100

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aa016d0
return false when deleting nse
Apr 22, 2024
10309d4
Merge branch 'main' into issue-3092
Apr 23, 2024
50ef202
Merge branch 'main' into issue-3092
Apr 24, 2024
0d8cdce
Merge branch 'main' into issue-3092
May 9, 2024
505e135
fix comments
May 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion core/src/main/java/com/datastrato/gravitino/storage/relational/RelationalEntityStore.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,11 @@ public <E extends Entity & HasIdentifier> E get(
@Override
public boolean delete(NameIdentifier ident, Entity.EntityType entityType, boolean cascade)
throws IOException {
return backend.delete(ident, entityType, cascade);
try {
return backend.delete(ident, entityType, cascade);
} catch (NoSuchEntityException nse) {
return false;
}
}

@Override
Expand Down
35 changes: 35 additions & 0 deletions core/src/test/java/com/datastrato/gravitino/storage/TestEntityStorage.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1277,20 +1277,27 @@ private void validateDeleteTopicCascade(EntityStore store, TopicEntity topic1)
// Delete the topic 'metalake.catalog.schema1.topic1'
Assertions.assertTrue(store.delete(topic1.nameIdentifier(), Entity.EntityType.TOPIC));
Assertions.assertFalse(store.exists(topic1.nameIdentifier(), Entity.EntityType.TOPIC));
// Delete again should return false
Assertions.assertFalse(store.delete(topic1.nameIdentifier(), Entity.EntityType.TOPIC));
}

private void validateDeleteFilesetCascade(EntityStore store, FilesetEntity fileset1)
throws IOException {
// Delete the fileset 'metalake.catalog.schema1.fileset1'
Assertions.assertTrue(store.delete(fileset1.nameIdentifier(), Entity.EntityType.FILESET, true));
Assertions.assertFalse(store.exists(fileset1.nameIdentifier(), Entity.EntityType.FILESET));
// Delete again should return false
Assertions.assertFalse(
store.delete(fileset1.nameIdentifier(), Entity.EntityType.FILESET, true));
}

private void validateDeleteTableCascade(EntityStore store, TableEntity table1)
throws IOException {
// Delete the table 'metalake.catalog.schema1.table1'
Assertions.assertTrue(store.delete(table1.nameIdentifier(), Entity.EntityType.TABLE, true));
Assertions.assertFalse(store.exists(table1.nameIdentifier(), Entity.EntityType.TABLE));
// Delete again should return false
Assertions.assertFalse(store.delete(table1.nameIdentifier(), Entity.EntityType.TABLE, true));
}

private void validateDeleteFileset(
Expand All @@ -1304,6 +1311,9 @@ private void validateDeleteFileset(
store.delete(fileset1InSchema2.nameIdentifier(), Entity.EntityType.FILESET));
Assertions.assertFalse(
store.exists(fileset1InSchema2.nameIdentifier(), Entity.EntityType.FILESET));
// Delete again should return false
Assertions.assertFalse(
store.delete(fileset1InSchema2.nameIdentifier(), Entity.EntityType.FILESET));

// Make sure fileset 'metalake.catalog.schema1.fileset1' still exist;
Assertions.assertEquals(
Expand All @@ -1320,6 +1330,8 @@ private void validateDeleteTopic(
// Delete the topic 'metalake.catalog.schema2.topic1'
Assertions.assertTrue(store.delete(topic1InSchema2.nameIdentifier(), Entity.EntityType.TOPIC));
Assertions.assertFalse(store.exists(topic1InSchema2.nameIdentifier(), Entity.EntityType.TOPIC));
// Delete again should return false
Assertions.assertFalse(store.delete(topic1InSchema2.nameIdentifier(), Entity.EntityType.TOPIC));

// Make sure topic 'metalake.catalog.schema1.topic1' still exist;
Assertions.assertEquals(
Expand Down Expand Up @@ -1352,6 +1364,10 @@ private void validateDeleteMetalakeCascade(
Assertions.assertFalse(store.exists(userNew.nameIdentifier(), Entity.EntityType.USER));
Assertions.assertFalse(store.exists(groupNew.nameIdentifier(), EntityType.GROUP));
Assertions.assertFalse(store.exists(roleNew.nameIdentifier(), EntityType.ROLE));

// Delete again should return false
Assertions.assertFalse(
store.delete(metalake.nameIdentifier(), Entity.EntityType.METALAKE, true));
}

private void validateDeleteCatalogCascade(
Expand All @@ -1368,6 +1384,8 @@ private void validateDeleteCatalogCascade(
Assertions.assertThrowsExactly(
NoSuchEntityException.class,
() -> store.get(schema2.nameIdentifier(), Entity.EntityType.SCHEMA, SchemaEntity.class));
// Delete again should return false
Assertions.assertFalse(store.delete(catalog.nameIdentifier(), Entity.EntityType.CATALOG, true));
}

private void validateDeleteSchemaCascade(
Expand Down Expand Up @@ -1419,6 +1437,8 @@ private void validateDeleteSchemaCascade(
Assertions.assertTrue(e instanceof NoSuchEntityException);
Assertions.assertTrue(e.getMessage().contains("schema1"));
}
// Delete again should return false
Assertions.assertFalse(store.delete(schema1.nameIdentifier(), Entity.EntityType.SCHEMA, true));

Assertions.assertThrows(
NoSuchEntityException.class,
Expand Down Expand Up @@ -1453,6 +1473,8 @@ private static void validateDeleteMetalake(
Assertions.assertFalse(store.exists(user2.nameIdentifier(), Entity.EntityType.USER));
Assertions.assertFalse(store.exists(group2.nameIdentifier(), Entity.EntityType.GROUP));
Assertions.assertFalse(store.exists(role2.nameIdentifier(), Entity.EntityType.ROLE));
// Delete again should return false
Assertions.assertFalse(store.delete(metalake.nameIdentifier(), Entity.EntityType.METALAKE));
}

private static void validateDeleteCatalog(
Expand Down Expand Up @@ -1488,6 +1510,8 @@ private static void validateDeleteCatalog(

store.delete(catalog.nameIdentifier(), Entity.EntityType.CATALOG);
Assertions.assertFalse(store.exists(catalog.nameIdentifier(), Entity.EntityType.CATALOG));
// Delete again should return false
Assertions.assertFalse(store.delete(catalog.nameIdentifier(), Entity.EntityType.CATALOG));
}

private static void validateDeleteSchema(
Expand Down Expand Up @@ -1521,6 +1545,13 @@ private static void validateDeleteSchema(
Assertions.assertFalse(store.exists(fileset1.nameIdentifier(), Entity.EntityType.FILESET));
Assertions.assertFalse(store.exists(topic1.nameIdentifier(), Entity.EntityType.TOPIC));
Assertions.assertFalse(store.exists(schema1.nameIdentifier(), Entity.EntityType.SCHEMA));

// Delete again should return false
Assertions.assertFalse(store.delete(table1.nameIdentifier(), Entity.EntityType.TABLE));
Assertions.assertFalse(store.delete(fileset1.nameIdentifier(), Entity.EntityType.FILESET));
Assertions.assertFalse(store.delete(topic1.nameIdentifier(), Entity.EntityType.TOPIC));
Assertions.assertFalse(store.delete(schema1.nameIdentifier(), Entity.EntityType.SCHEMA));

// Now we re-insert schema1, table1, fileset1 and topic1, and everything should be OK
SchemaEntity schema1New =
createSchemaEntity(
Expand Down Expand Up @@ -1567,6 +1598,8 @@ private void validateDeleteUser(EntityStore store, UserEntity user1) throws IOEx
Assertions.assertTrue(store.exists(user1.nameIdentifier(), Entity.EntityType.USER));
Assertions.assertTrue(store.delete(user1.nameIdentifier(), Entity.EntityType.USER));
Assertions.assertFalse(store.exists(user1.nameIdentifier(), Entity.EntityType.USER));
// delete again should return false
Assertions.assertFalse(store.delete(user1.nameIdentifier(), Entity.EntityType.USER));
xloya marked this conversation as resolved.
Show resolved Hide resolved

UserEntity user =
createUser(RandomIdGenerator.INSTANCE.nextId(), "metalake", "user1", user1.auditInfo());
Expand Down Expand Up @@ -1600,6 +1633,8 @@ private void validateDeleteTable(
// Delete the table 'metalake.catalog.schema2.table1'
Assertions.assertTrue(store.delete(table1InSchema2.nameIdentifier(), Entity.EntityType.TABLE));
Assertions.assertFalse(store.exists(table1InSchema2.nameIdentifier(), Entity.EntityType.TABLE));
// delete again should return false
Assertions.assertFalse(store.delete(table1InSchema2.nameIdentifier(), Entity.EntityType.TABLE));

// Make sure table 'metalake.catalog.schema1.table1' still exist;
Assertions.assertEquals(
Expand Down
Loading