[Bug report] Dropping an Iceberg schema is misunderstanding

[yuqi1129]

Describe what's wrong

Error message and/or stacktrace

Caused by: java.lang.IllegalArgumentException: Iceberg does not support cascading delete operations.
	at com.google.common.base.Preconditions.checkArgument(Preconditions.java:145) ~[guava-31.1-jre.jar:?]
	at com.datastrato.gravitino.catalog.lakehouse.iceberg.IcebergCatalogOperations.dropSchema(IcebergCatalogOperations.java:304) ~[?:?]
	at com.datastrato.gravitino.catalog.CatalogOperationDispatcher.lambda$null$17(CatalogOperationDispatcher.java:289) ~[gravitino-core-0.4.0-SNAPSHOT.jar:?]
	at com.datastrato.gravitino.catalog.CatalogManager$CatalogWrapper.lambda$doWithSchemaOps$0(CatalogManager.java:86) ~[gravitino-core-0.4.0-SNAPSHOT.jar:?]
	at com.datastrato.gravitino.utils.IsolatedClassLoader.withClassLoader(IsolatedClassLoader.java:69) ~[gravitino-core-0.4.0-SNAPSHOT.jar:?]
	at com.datastrato.gravitino.catalog.CatalogManager$CatalogWrapper.doWithSchemaOps(CatalogManager.java:81) ~[gravitino-core-0.4.0-SNAPSHOT.jar:?]
	at com.datastrato.gravitino.catalog.CatalogOperationDispatcher.lambda$dropSchema$18(CatalogOperationDispatcher.java:289) ~[gravitino-core-0.4.0-SNAPSHOT.jar:?]
	at com.datastrato.gravitino.catalog.CatalogOperationDispatcher.doWithCatalog(CatalogOperationDispatcher.java:681) ~[gravitino-core-0.4.0-SNAPSHOT.jar:?]
	... 67 more

How to reproduce

Execute drop schema db1 CASCADE to drop Iceberg schema.

Additional context

No response

[yuqi1129]

The root cause:
Although Gravitino made an exception, it caught the exception and returns false, Trino can't detect whether we have succeed in dropping it by it.

[jerryshao]

So I think you should determine this based on the return value.

[yuqi1129]

So I think you should determine this based on the return value.

We are unable to determine the outcome based solely on the return value. For example. A false return value may indicate

• The schema you dropped does not exist.
• Something went wrong with the server.
• Others.

So, if we need to determine the cause, I think we should change the return type of the method. A simple boolean type cannot meet our needs. Do you mean to change the return type to determine this based on the return value?

[jerryshao]

We don't need to determine the cause. A typical dropXXX API design I saw in several projects doesn't throw exceptions, some even don't have a return value (HMS).

[yuqi1129]

We don't need to determine the cause. A typical dropXXX API design I saw in several projects doesn't throw exceptions, some even don't have a return value (HMS).

If so, could you offer some suggestions on how to handle this situation here? I need to obtain accurate information about what's wrong.

Though the return value false can handle this case, but no more details can be shown here.

[jerryshao]

I'm curious why do you need to know the failure reason, from Trino side, "false" means the drop operation is failed, the schema still exists.

Only "true" means drop operation is successful, otherwise it will be false no matter what reason it is. From Trino side, using the return value should be enough.

Unless one scenario that the return value is "true" but schema exists, that is an unexpected scenario we should fix.

[yuqi1129]

Only "true" means drop operation is successful, otherwise it will be false no matter what reason it is. From Trino side, using the return value should be enough.

True or false is sufficient for Trino program logic, but not for users. Users who use the drop operation failed, but without any information about why it failed, they may be confused and unsure of what to do. Users should be aware of the primary cause.

Besides, I see other interfaces like loadTable, createTable, and alterTable also make the error messages public NOT just swallow them.