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

[Bug report] Drop schema with false cascade goes unexpected #1231

Closed
yuqi1129 opened this issue Dec 21, 2023 · 3 comments · Fixed by #1235
Closed

[Bug report] Drop schema with false cascade goes unexpected #1231

yuqi1129 opened this issue Dec 21, 2023 · 3 comments · Fixed by #1235
Assignees

Comments

@yuqi1129
Copy link
Contributor

Describe what's wrong

Dropping a schema was unsuccessful, but it did indeed drop.

Error message and/or stacktrace

curl -X DELETE -H "Content-Type: application/json" http://localhost:8090/api/metalakes/test/catalogs/mysql_catalogs/schemas/db4?cascade=false
{"code":1002,"type":"RuntimeException","message":"Failed to operate object [db4] operation [DROP] under [mysql_catalogs], reason [RuntimeException]","stack":["java.lang.RuntimeException: com.datastrato.gravitino.exceptions.NonEmptyEntityException: Entity test.mysql_catalogs.db4 has sub-entities, you should remove sub-entities first","\tat com.datastrato.gravitino.catalog.CatalogOperationDispatcher.dropSchema(CatalogOperationDispatcher.java:299)","\tat com.datastrato.gravitino.server.web.rest.SchemaOperations.dropSchema(SchemaOperations.java:149)","\tat java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)","\tat java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)","\tat java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)","\tat java.base/java.lang.reflect.Method.invoke(Method.java:568)","\tat org.glassfish.jersey.server.model.internal.ResourceMethodInvocationHandlerFactory.lambda$static$0(ResourceMethodInvocationHandlerFactory.java:52)","\tat org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher$1.run(AbstractJavaResourceMethodDispatcher.java:134)","\tat org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.invoke(AbstractJavaResourceMethodDispatcher.java:177)","\tat org.glassfish.jersey.server.model.internal.JavaResourceMethodDispatcherProvider$ResponseOutInvoker.doDispatch(JavaResourceMethodDispatcherProvider.java:176)","\tat org.glassfish.jersey.server.model.internal.AbstractJavaResourceMethodDispatcher.dispatch(AbstractJavaResourceMethodDispatcher.java:81)","\tat org.glassfish.jersey.server.model.ResourceMethodInvoker.invoke(ResourceMethodInvoker.java:478)","\tat org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:400)","\tat org.glassfish.jersey.server.model.ResourceMethodInvoker.apply(ResourceMethodInvoker.java:81)","\tat org.glassfish.jersey.server.ServerRuntime$1.run(ServerRuntime.java:256)","\tat org.glassfish.jersey.internal.Errors$1.call(Errors.java:248)","\tat org.glassfish.jersey.internal.Errors$1.call(Errors.java:244)","\tat org.glassfish.jersey.internal.Errors.process(Errors.java:292)","\tat org.glassfish.jersey.internal.Errors.process(Errors.java:274)","\tat org.glassfish.jersey.internal.Errors.process(Errors.java:244)","\tat org.glassfish.jersey.process.internal.RequestScope.runInScope(RequestScope.java:265)","\tat org.glassfish.jersey.server.ServerRuntime.process(ServerRuntime.java:235)","\tat org.glassfish.jersey.server.ApplicationHandler.handle(ApplicationHandler.java:684)","\tat org.glassfish.jersey.servlet.WebComponent.serviceImpl(WebComponent.java:394)","\tat org.glassfish.jersey.servlet.WebComponent.service(WebComponent.java:346)","\tat org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:358)","\tat org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:311)","\tat org.glassfish.jersey.servlet.ServletContainer.service(ServletContainer.java:205)","\tat org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:799)","\tat org.eclipse.jetty.servlet.ServletHandler$ChainEnd.doFilter(ServletHandler.java:1656)","\tat com.datastrato.gravitino.server.auth.AuthenticationFilter.doFilter(AuthenticationFilter.java:57)","\tat org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)","\tat org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1626)","\tat com.datastrato.gravitino.server.web.VersioningFilter.doFilter(VersioningFilter.java:107)","\tat org.eclipse.jetty.servlet.FilterHolder.doFilter(FilterHolder.java:193)","\tat org.eclipse.jetty.servlet.ServletHandler$Chain.doFilter(ServletHandler.java:1626)","\tat org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:552)","\tat org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)","\tat org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:600)","\tat org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)","\tat org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:235)","\tat org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:1624)","\tat org.eclipse.jetty.server.handler.ScopedHandler.nextHandle(ScopedHandler.java:233)","\tat org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1440)","\tat org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:188)","\tat org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:505)","\tat org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:1594)","\tat org.eclipse.jetty.server.handler.ScopedHandler.nextScope(ScopedHandler.java:186)","\tat org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1355)","\tat org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)","\tat org.eclipse.jetty.server.handler.HandlerCollection.handle(HandlerCollection.java:146)","\tat org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:127)","\tat org.eclipse.jetty.server.Server.handle(Server.java:516)","\tat org.eclipse.jetty.server.HttpChannel.lambda$handle$1(HttpChannel.java:487)","\tat org.eclipse.jetty.server.HttpChannel.dispatch(HttpChannel.java:732)","\tat org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:479)","\tat org.eclipse.jetty.server.HttpConnection.onFillable(HttpConnection.java:277)","\tat org.eclipse.jetty.io.AbstractConnection$ReadCallback.succeeded(AbstractConnection.java:311)","\tat org.eclipse.jetty.io.FillInterest.fillable(FillInterest.java:105)","\tat org.eclipse.jetty.io.ChannelEndPoint$1.run(ChannelEndPoint.java:104)","\tat org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.runTask(EatWhatYouKill.java:338)","\tat org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.doProduce(EatWhatYouKill.java:315)","\tat org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.tryProduce(EatWhatYouKill.java:173)","\tat org.eclipse.jetty.util.thread.strategy.EatWhatYouKill.run(EatWhatYouKill.java:131)","\tat org.eclipse.jetty.util.thread.ReservedThreadExecutor$ReservedThread.run(ReservedThreadExecutor.java:409)","\tat org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:883)","\tat org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1034)","\tat java.base/java.lang.Thread.run(Thread.java:833)","Caused by: com.datastrato.gravitino.exceptions.NonEmptyEntityException: Entity test.mysql_catalogs.db4 has sub-entities, you should remove sub-entities first","\tat com.datastrato.gravitino.storage.kv.KvEntityStore.lambda$null$10(KvEntityStore.java:384)","\tat com.datastrato.gravitino.storage.FunctionUtils.executeInTransaction(FunctionUtils.java:55)","\tat com.datastrato.gravitino.storage.kv.KvEntityStore.executeInTransaction(KvEntityStore.java:406)","\tat com.datastrato.gravitino.storage.kv.KvEntityStore.lambda$delete$11(KvEntityStore.java:357)","\tat com.datastrato.gravitino.storage.FunctionUtils.executeWithWriteLock(FunctionUtils.java:43)","\tat com.datastrato.gravitino.storage.kv.KvEntityStore.delete(KvEntityStore.java:355)","\tat com.datastrato.gravitino.catalog.CatalogOperationDispatcher.dropSchema(CatalogOperationDispatcher.java:297)","\t... 67 more"]}%

How to reproduce

  • Main branch

Additional context

No response

@yuqi1129
Copy link
Contributor Author

yuqi1129 commented Dec 21, 2023

@Clearvive

  @Override
  public String generateDropDatabaseSql(String databaseName, boolean cascade) {
    if (cascade) {
      throw new UnsupportedOperationException(
          "MySQL does not support CASCADE option for DROP DATABASE.");
    }
    return "DROP DATABASE `" + databaseName + "`";
  }

I think we should change the logic here.

  • If cascade is true, then goes ahead
  • If cascade is false, then we should judge whether there are tables under the database.

EntityStore obeys the logic above, or we can't guarantee consistency between EntityStore and underlying storage.

@yuqi1129
Copy link
Contributor Author

yuqi1129 commented Dec 22, 2023

Root cause:
The cascade is false.

Dropping a database in MySQL with cascade false succeeds and fails in KvEntityStore, we should set cascade to true when dropping a schema with tables under it in KvEntityStore.

@Clearvive
Copy link
Contributor

I will fix this issue

jerryshao pushed a commit that referenced this issue Dec 25, 2023
### What changes were proposed in this pull request?

- Allow dropping a database when `cascase` is `true`.
- If the database is not empty and `cascade` is `false`, disable the
drop operation.

### Why are the changes needed?

Align the logic with Gravitino EntityStore.

Fix: #1231 

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

Add UT `testDropMySQLDatabase`
github-actions bot pushed a commit that referenced this issue Dec 25, 2023
### What changes were proposed in this pull request?

- Allow dropping a database when `cascase` is `true`.
- If the database is not empty and `cascade` is `false`, disable the
drop operation.

### Why are the changes needed?

Align the logic with Gravitino EntityStore.

Fix: #1231 

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

Add UT `testDropMySQLDatabase`
jerryshao pushed a commit that referenced this issue Dec 25, 2023
### What changes were proposed in this pull request?

- Allow dropping a database when `cascase` is `true`.
- If the database is not empty and `cascade` is `false`, disable the
drop operation.

### Why are the changes needed?

Align the logic with Gravitino EntityStore.

Fix: #1231 

### Does this PR introduce _any_ user-facing change?

N/A

### How was this patch tested?

Add UT `testDropMySQLDatabase`

Co-authored-by: Qi Yu <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants