geyanggang commented on code in PR #14663:
URL: https://github.com/apache/gravitino/pull/14663#discussion_r4488524399
##########
trino-connector/src/common/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -312,8 +312,11 @@ public void dropSchema(String schemaName, boolean cascade)
{
try {
success = schemaCatalog.dropSchema(schemaName, cascade);
} catch (NonEmptySchemaException e) {
- throw new TrinoException(
- GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EMPTY, "Schema does not
empty", e);
+ // Keep the server's reason (for example a blocking dependency on a
cascade drop) instead of a
+ // fixed text, so the user sees why the drop was refused.
+ String reason =
+ Strings.isNullOrEmpty(e.getMessage()) ? "Schema is not empty" :
e.getMessage();
Review Comment:
Thanks @diqiu50, good catch on both points.
- `getMessage()` of a REST failure carries the server stack trace, so
`NonEmptySchemaException` now goes through `toTrinoException` (the code stays
`GRAVITINO_SCHEMA_NOT_EMPTY`, the message is the first line). The unit tests
use a REST-shaped multi-line message on cascade and non-cascade drops.
- You were also right about PostgreSQL: on a real server the first line only
says "other objects depend on it", and the dependent object is in the later
`DETAIL` line. `PostgreSqlSchemaOperations` now folds the reason and its detail
into one line, with a Docker test that checks a view in another schema is named
on that line.
I confirmed the new tests fail against the previous behavior and pass now.
Pushed in 30ed419f5.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
