This is an automated email from the ASF dual-hosted git repository.
diqiu50 pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/gravitino.git
The following commit(s) were added to refs/heads/main by this push:
new 880032f6af [#13782] fix(trino-connector): Report authorization and
validation refusals as user errors (#13787)
880032f6af is described below
commit 880032f6af69a997da5ffde82fddf3574c867d51
Author: Yuhui <[email protected]>
AuthorDate: Tue Oct 13 21:44:35 2026 +0800
[#13782] fix(trino-connector): Report authorization and validation refusals
as user errors (#13787)
### What changes were proposed in this pull request?
Classify failures from the Gravitino server in the Trino connector
through a single
`GravitinoErrorCode.toTrinoException`, used by
`CatalogConnectorMetadata`, the catalog
stored procedures and per-user client creation:
- `ForbiddenException` -> `PERMISSION_DENIED`
- `NotInUseException` -> `CATALOG_UNAVAILABLE`
- `ConnectionFailedException`, or a REST failure caused by a network
error -> `GRAVITINO_CONNECTION_FAILED` (new)
- `IllegalArgumentException` -> `GRAVITINO_ILLEGAL_ARGUMENT`
- `UnsupportedOperationException` -> `GRAVITINO_UNSUPPORTED_OPERATION`
- `TrinoException` passes through; others -> `GENERIC_INTERNAL_ERROR`
Messages keep only the first line, dropping the server stack trace. Also
fix the exception
types caught by `createSchema`.
### Why are the changes needed?
Expected refusals and connection failures were reported as
`GENERIC_INTERNAL_ERROR`, so they
looked like Trino faults.
Fix: #13782
### Does this PR introduce _any_ user-facing change?
Yes, these failures use the error codes above, and catalog procedure
errors keep their own codes
instead of `GRAVITINO_UNSUPPORTED_OPERATION`.
### How was this patch tested?
Added `TestGravitinoErrorCode` and `TestCatalogConnectorMetadataErrors`,
and extended the
stored procedure and forward-user tests.
---
.../testsets/jdbc-mysql/00005_create_catalog.txt | 2 +-
.../trino/connector/GravitinoConnector.java | 32 +--
.../trino/connector/GravitinoErrorCode.java | 94 ++++++++-
.../connector/catalog/CatalogConnectorManager.java | 3 +-
.../catalog/CatalogConnectorMetadata.java | 76 +++++--
.../AlterCatalogStoredProcedure.java | 7 +-
.../CreateCatalogStoredProcedure.java | 16 +-
.../DropCatalogStoredProcedure.java | 10 +-
.../trino/connector/TestGravitinoConnector.java | 5 +-
.../TestGravitinoConnectorForwardUser.java | 24 +++
.../trino/connector/TestGravitinoErrorCode.java | 167 +++++++++++++++
.../TestCatalogConnectorMetadataErrors.java | 223 +++++++++++++++++++++
.../TestAlterCatalogStoredProcedure.java | 26 +++
.../TestCreateCatalogStoredProcedure.java | 85 ++++++++
.../TestDropCatalogStoredProcedure.java | 29 ++-
15 files changed, 718 insertions(+), 81 deletions(-)
diff --git
a/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00005_create_catalog.txt
b/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00005_create_catalog.txt
index d17eaa68f9..6e75e27dd3 100644
---
a/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00005_create_catalog.txt
+++
b/trino-connector/integration-test/src/test/resources/trino-ci-testset/testsets/jdbc-mysql/00005_create_catalog.txt
@@ -10,7 +10,7 @@ CALL
<BLANK_LINE>
-<QUERY_FAILED> Drop catalog failed. Catalog test.gt_mysql_xxx1 not exists
+<QUERY_FAILED> Catalog test.gt_mysql_xxx1 not exists
CALL
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
index 0eb06e1495..8f130f2974 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoConnector.java
@@ -280,7 +280,19 @@ public class GravitinoConnector implements Connector {
return perUserSessionCache.get(
credKey,
() -> {
- GravitinoAdminClient userClient = buildAuthClient(session);
+ GravitinoAdminClient userClient;
+ try {
+ userClient = buildAuthClient(session);
+ } catch (IllegalArgumentException |
UnsupportedOperationException e) {
+ // buildAuthClient raises these for an invalid or
unsupported credential.
+ throw new TrinoException(
+ PERMISSION_DENIED,
+ "Failed to authenticate user '"
+ + session.getUser()
+ + "' with Gravitino: "
+ + e.getMessage(),
+ e);
+ }
GravitinoMetalake userMetalake =
userClient.loadMetalake(catalogConnectorContext.getMetalake().name());
return new UserSession(
@@ -291,20 +303,10 @@ public class GravitinoConnector implements Connector {
Throwable cause = e.getCause() == null ? e : e.getCause();
LOG.warn(
cause, "Failed to create per-user Gravitino client for user '%s'",
session.getUser());
- if (cause instanceof TrinoException) {
- // Already carries a specific Trino error code (e.g. from
buildForSession); re-wrapping
- // would swallow it.
- throw (TrinoException) cause;
- }
- if (cause instanceof IllegalArgumentException
- || cause instanceof UnsupportedOperationException) {
- throw new TrinoException(
- PERMISSION_DENIED,
- "Failed to authenticate user '"
- + session.getUser()
- + "' with Gravitino: "
- + cause.getMessage(),
- cause);
+ if (cause instanceof Exception) {
+ throw GravitinoErrorCode.toTrinoException(
+ (Exception) cause,
+ "Failed to create per-user Gravitino client for user '" +
session.getUser() + "'");
}
throw new TrinoException(
GENERIC_INTERNAL_ERROR,
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoErrorCode.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoErrorCode.java
index 059d53cb2b..9d48feb2d0 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoErrorCode.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoErrorCode.java
@@ -19,11 +19,24 @@
package org.apache.gravitino.trino.connector;
import static io.trino.spi.ErrorType.EXTERNAL;
+import static io.trino.spi.StandardErrorCode.CATALOG_UNAVAILABLE;
+import static io.trino.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR;
+import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
import io.trino.spi.ErrorCode;
import io.trino.spi.ErrorCodeSupplier;
import io.trino.spi.ErrorType;
-import java.util.List;
+import io.trino.spi.TrinoException;
+import java.io.InterruptedIOException;
+import java.net.SocketException;
+import java.net.UnknownHostException;
+import java.util.concurrent.ExecutionException;
+import javax.annotation.Nullable;
+import org.apache.commons.lang3.StringUtils;
+import org.apache.gravitino.exceptions.ConnectionFailedException;
+import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.exceptions.NotInUseException;
+import org.apache.gravitino.exceptions.RESTException;
/** Gravitino error code. */
public enum GravitinoErrorCode implements ErrorCodeSupplier {
@@ -78,7 +91,9 @@ public enum GravitinoErrorCode implements ErrorCodeSupplier {
/** Duplicated catalogs. */
GRAVITINO_DUPLICATED_CATALOGS(24, EXTERNAL),
/** Expression error. */
- GRAVITINO_EXPRESSION_ERROR(25, EXTERNAL);
+ GRAVITINO_EXPRESSION_ERROR(25, EXTERNAL),
+ /** Failed to connect to the Gravitino server or to the underlying source of
a catalog. */
+ GRAVITINO_CONNECTION_FAILED(26, EXTERNAL);
// suppress ImmutableEnumChecker because ErrorCode is outside the project.
@SuppressWarnings("ImmutableEnumChecker")
@@ -94,15 +109,74 @@ public enum GravitinoErrorCode implements
ErrorCodeSupplier {
}
/**
- * Converts an exception's message to a simplified error message. If the
message contains multiple
- * lines, returns the first two lines concatenated. If the message contains
only one line, returns
- * that line.
+ * Converts an exception into a Trino exception. See {@link
#toTrinoException(Exception, String)}.
*
- * @param e the exception whose message should be simplified
- * @return the simplified error message
+ * @param e the exception to convert
+ * @return the Trino exception to throw
*/
- public static String toSimpleErrorMessage(Exception e) {
- List<String> lines = e.getMessage().lines().toList();
- return lines.size() > 1 ? lines.get(0) + lines.get(1) : lines.get(0);
+ public static TrinoException toTrinoException(Exception e) {
+ return toTrinoException(e, null);
+ }
+
+ /**
+ * Converts an exception into a Trino exception. A {@link TrinoException} is
returned as is, so a
+ * caller may route every failure of a guarded block through this method,
including a {@link
+ * TrinoException} it throws inside that block. An {@link
ExecutionException} is classified by its
+ * cause. {@link ForbiddenException}, {@link NotInUseException}, {@link
IllegalArgumentException}
+ * and {@link UnsupportedOperationException} map to {@code
PERMISSION_DENIED}, {@code
+ * CATALOG_UNAVAILABLE}, {@link #GRAVITINO_ILLEGAL_ARGUMENT} and {@link
+ * #GRAVITINO_UNSUPPORTED_OPERATION}. A {@link ConnectionFailedException},
or a {@link
+ * RESTException} caused by a network failure, maps to {@link
#GRAVITINO_CONNECTION_FAILED}.
+ * Anything else maps to {@code GENERIC_INTERNAL_ERROR}.
+ *
+ * @param exception the exception to convert
+ * @param context describes the failed operation; it prefixes the message of
an exception that
+ * maps to {@code GENERIC_INTERNAL_ERROR}, whose own message may not say
what failed
+ * @return the Trino exception to throw
+ */
+ public static TrinoException toTrinoException(Exception exception, @Nullable
String context) {
+ Exception e =
+ exception instanceof ExecutionException && exception.getCause()
instanceof Exception
+ ? (Exception) exception.getCause()
+ : exception;
+ if (e instanceof TrinoException) {
+ return (TrinoException) e;
+ }
+ if (e instanceof ForbiddenException) {
+ return new TrinoException(PERMISSION_DENIED, firstLineMessage(e), e);
+ }
+ if (e instanceof NotInUseException) {
+ return new TrinoException(CATALOG_UNAVAILABLE, firstLineMessage(e), e);
+ }
+ if (e instanceof ConnectionFailedException
+ || (e instanceof RESTException && isNetworkFailure(e.getCause()))) {
+ return new TrinoException(GRAVITINO_CONNECTION_FAILED,
firstLineMessage(e), e);
+ }
+ if (e instanceof IllegalArgumentException) {
+ return new TrinoException(GRAVITINO_ILLEGAL_ARGUMENT,
firstLineMessage(e), e);
+ }
+ if (e instanceof UnsupportedOperationException) {
+ return new TrinoException(GRAVITINO_UNSUPPORTED_OPERATION,
firstLineMessage(e), e);
+ }
+ String message = firstLineMessage(e);
+ return new TrinoException(
+ GENERIC_INTERNAL_ERROR, context == null ? message : context + ": " +
message, e);
+ }
+
+ // The REST client wraps every IOException in a RESTException, including
JSON (de)serialization
+ // failures, so only socket-level causes mean the server could not be
reached.
+ private static boolean isNetworkFailure(@Nullable Throwable cause) {
+ return cause instanceof SocketException
+ || cause instanceof UnknownHostException
+ || cause instanceof InterruptedIOException;
+ }
+
+ // Keeps the first non-blank line of the message. A failure received over
REST carries the server
+ // stack trace after it, and other failures may add details there as well;
the full text stays
+ // available in the cause.
+ private static String firstLineMessage(Exception e) {
+ return StringUtils.isBlank(e.getMessage())
+ ? e.toString()
+ :
e.getMessage().lines().filter(StringUtils::isNotBlank).findFirst().orElse(e.toString());
}
}
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorManager.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorManager.java
index 1446d97dbc..ab9b3e80d8 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorManager.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorManager.java
@@ -409,8 +409,7 @@ public class CatalogConnectorManager {
// Report the root cause: the actual reason a registration failed, such as
"Access Denied:
// Cannot create catalog", is wrapped in several layers of TrinoException
by the time it gets
// here, and the outer messages say nothing a user can act on. The
outermost message is kept
- // as a prefix because it names the subsystem that failed. Do not use
- // GravitinoErrorCode.toSimpleErrorMessage(), it throws on an exception
with no message.
+ // as a prefix because it names the subsystem that failed.
Throwable rootCause;
try {
rootCause = Throwables.getRootCause(e);
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java
index 4e71662872..77c2e2a4c2 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java
@@ -34,9 +34,11 @@ import org.apache.gravitino.Schema;
import org.apache.gravitino.SupportsSchemas;
import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
+import org.apache.gravitino.exceptions.NoSuchFunctionException;
import org.apache.gravitino.exceptions.NoSuchSchemaException;
import org.apache.gravitino.exceptions.NoSuchTableException;
import org.apache.gravitino.exceptions.NonEmptySchemaException;
+import org.apache.gravitino.exceptions.SchemaAlreadyExistsException;
import org.apache.gravitino.exceptions.TableAlreadyExistsException;
import org.apache.gravitino.function.Function;
import org.apache.gravitino.function.FunctionCatalog;
@@ -90,6 +92,8 @@ public class CatalogConnectorMetadata {
GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
"Catalog does not support schema or table operations." +
e.getMessage(),
e);
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
}
}
@@ -104,6 +108,8 @@ public class CatalogConnectorMetadata {
} catch (NoSuchCatalogException e) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_CATALOG_NOT_EXISTS,
CATALOG_DOES_NOT_EXIST_MSG, e);
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
}
}
@@ -112,7 +118,7 @@ public class CatalogConnectorMetadata {
*
* @param schemaName the name of the schema
* @return the Gravitino schema
- * @throws TrinoException if the schema is not found
+ * @throws TrinoException if the schema is not found or the request fails
*/
public GravitinoSchema getSchema(String schemaName) {
try {
@@ -121,6 +127,8 @@ public class CatalogConnectorMetadata {
} catch (NoSuchSchemaException e) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS,
SCHEMA_DOES_NOT_EXIST_MSG, e);
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
}
}
@@ -130,7 +138,7 @@ public class CatalogConnectorMetadata {
* @param schemaName the name of the schema
* @param tableName the name of the table
* @return the Gravitino table
- * @throws TrinoException if the table is not found
+ * @throws TrinoException if the table is not found or the request fails
*/
public GravitinoTable getTable(String schemaName, String tableName) {
try {
@@ -139,6 +147,8 @@ public class CatalogConnectorMetadata {
} catch (NoSuchTableException e) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_TABLE_NOT_EXISTS, "Table does not
exist", e);
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
}
}
@@ -155,6 +165,8 @@ public class CatalogConnectorMetadata {
} catch (NoSuchSchemaException e) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS,
SCHEMA_DOES_NOT_EXIST_MSG, e);
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
}
}
@@ -166,7 +178,11 @@ public class CatalogConnectorMetadata {
* @return true if the table exists, false otherwise
*/
public boolean tableExists(String schemaName, String tableName) {
- return tableCatalog.tableExists(NameIdentifier.of(schemaName, tableName));
+ try {
+ return tableCatalog.tableExists(NameIdentifier.of(schemaName,
tableName));
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
+ }
}
/**
@@ -195,6 +211,8 @@ public class CatalogConnectorMetadata {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_TABLE_ALREADY_EXISTS, "Table already
exists", e);
}
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
}
}
@@ -206,12 +224,14 @@ public class CatalogConnectorMetadata {
public void createSchema(GravitinoSchema schema) {
try {
schemaCatalog.createSchema(schema.getName(), schema.getComment(),
schema.getProperties());
- } catch (NoSuchSchemaException e) {
+ } catch (NoSuchCatalogException e) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_CATALOG_NOT_EXISTS,
CATALOG_DOES_NOT_EXIST_MSG, e);
- } catch (TableAlreadyExistsException e) {
+ } catch (SchemaAlreadyExistsException e) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_SCHEMA_ALREADY_EXISTS, "Schema already
exists", e);
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
}
}
@@ -222,17 +242,18 @@ public class CatalogConnectorMetadata {
* @param cascade whether to cascade drop the schema
*/
public void dropSchema(String schemaName, boolean cascade) {
+ boolean success;
try {
- boolean success = schemaCatalog.dropSchema(schemaName, cascade);
-
- if (!success) {
- throw new TrinoException(
- GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS, "Drop schema
failed");
- }
-
+ success = schemaCatalog.dropSchema(schemaName, cascade);
} catch (NonEmptySchemaException e) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EMPTY, "Schema does not
empty", e);
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
+ }
+ if (!success) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS, "Drop schema
failed");
}
}
@@ -242,9 +263,14 @@ public class CatalogConnectorMetadata {
* @param tableName the name of the table
*/
public void dropTable(SchemaTableName tableName) {
- boolean dropped =
- tableCatalog.dropTable(
- NameIdentifier.of(tableName.getSchemaName(),
tableName.getTableName()));
+ boolean dropped;
+ try {
+ dropped =
+ tableCatalog.dropTable(
+ NameIdentifier.of(tableName.getSchemaName(),
tableName.getTableName()));
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
+ }
if (!dropped) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_OPERATION_FAILED, "Failed to drop table
" + tableName);
@@ -268,12 +294,8 @@ public class CatalogConnectorMetadata {
} catch (NoSuchTableException e) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_TABLE_NOT_EXISTS, "Table does not
exist");
- } catch (IllegalArgumentException e) {
- // TODO yuhui need improve get the error message. From
IllegalArgumentException.
- // At present, the IllegalArgumentException cannot get the error
information clearly from the
- // Gravitino server.
- String message = GravitinoErrorCode.toSimpleErrorMessage(e);
- throw new TrinoException(GravitinoErrorCode.GRAVITINO_ILLEGAL_ARGUMENT,
message, e);
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
}
}
@@ -436,6 +458,8 @@ public class CatalogConnectorMetadata {
} catch (NoSuchSchemaException e) {
throw new TrinoException(
GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS,
SCHEMA_DOES_NOT_EXIST_MSG, e);
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
}
}
@@ -446,11 +470,19 @@ public class CatalogConnectorMetadata {
* @param functionName the name of the function
* @return the function
* @throws UnsupportedOperationException if the catalog does not support
functions
+ * @throws NoSuchFunctionException if the function does not exist
*/
public Function getFunction(String schemaName, String functionName) {
if (!supportsFunctions()) {
throw new UnsupportedOperationException("Catalog does not support
functions");
}
- return functionCatalog.getFunction(NameIdentifier.of(schemaName,
functionName));
+ try {
+ return functionCatalog.getFunction(NameIdentifier.of(schemaName,
functionName));
+ } catch (NoSuchFunctionException e) {
+ // Callers treat a missing function as an empty result.
+ throw e;
+ } catch (RuntimeException e) {
+ throw GravitinoErrorCode.toTrinoException(e);
+ }
}
}
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/AlterCatalogStoredProcedure.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/AlterCatalogStoredProcedure.java
index f0dcf7cf0e..18d9c79ebe 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/AlterCatalogStoredProcedure.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/AlterCatalogStoredProcedure.java
@@ -34,7 +34,6 @@ import java.util.List;
import java.util.Map;
import java.util.Optional;
import javax.annotation.Nullable;
-import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.CatalogChange;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
@@ -173,10 +172,8 @@ public class AlterCatalogStoredProcedure extends
GravitinoStoredProcedure {
GravitinoErrorCode.GRAVITINO_CATALOG_NOT_EXISTS,
"Catalog " + NameIdentifier.of(metalake, catalogName) + " not
exists.");
} catch (Exception e) {
- throw new TrinoException(
- GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
- "alter catalog failed. " + (StringUtils.isEmpty(e.getMessage()) ? ""
: e.getMessage()),
- e);
+ throw GravitinoErrorCode.toTrinoException(
+ e, "Alter catalog " + NameIdentifier.of(metalake, catalogName) + "
failed");
}
}
}
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/CreateCatalogStoredProcedure.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/CreateCatalogStoredProcedure.java
index 405ef0ce73..74d26c6b50 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/CreateCatalogStoredProcedure.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/CreateCatalogStoredProcedure.java
@@ -101,7 +101,8 @@ public class CreateCatalogStoredProcedure extends
GravitinoStoredProcedure {
* @param properties the properties of the catalog
* @param ignoreExist whether to ignore if the catalog already exists
* @param metalakeArgument the metalake to create the catalog in, null to
use the configured one
- * @throws TrinoException if the catalog already exists and ignoreExist is
false
+ * @throws TrinoException if the catalog already exists and ignoreExist is
false, or the operation
+ * fails
*/
public void createCatalog(
String catalogName,
@@ -147,13 +148,14 @@ public class CreateCatalogStoredProcedure extends
GravitinoStoredProcedure {
"Catalog " + NameIdentifier.of(metalake, catalogName) + " already
exists in the server.");
} catch (Exception e) {
// The failure can come from a CREATE CATALOG the load loop ran, whose
message may carry the
- // credentials the statement embedded, so it is masked before reaching
the caller.
+ // credentials the statement embedded, so the message is masked while
the classified error
+ // code is kept.
+ TrinoException error =
+ GravitinoErrorCode.toTrinoException(
+ e, "Create catalog " + NameIdentifier.of(metalake, catalogName)
+ " failed");
throw new TrinoException(
- GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
- "Create catalog failed. "
- + (StringUtils.isEmpty(e.getMessage())
- ? ""
- : CatalogRegister.redactSecrets(e.getMessage())),
+ error::getErrorCode,
+
CatalogRegister.redactSecrets(StringUtils.defaultString(error.getMessage())),
e);
}
}
diff --git
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/DropCatalogStoredProcedure.java
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/DropCatalogStoredProcedure.java
index a813a35df1..6d2fc9234a 100644
---
a/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/DropCatalogStoredProcedure.java
+++
b/trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/system/storedprocedure/DropCatalogStoredProcedure.java
@@ -28,7 +28,6 @@ import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.util.List;
import javax.annotation.Nullable;
-import org.apache.commons.lang3.StringUtils;
import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.exceptions.NoSuchMetalakeException;
import org.apache.gravitino.trino.connector.GravitinoErrorCode;
@@ -88,7 +87,8 @@ public class DropCatalogStoredProcedure extends
GravitinoStoredProcedure {
* @param catalogName the name of the catalog to drop
* @param ignoreNotExist whether to ignore if the catalog does not exist
* @param metalakeArgument the metalake to drop the catalog from, null to
use the configured one
- * @throws TrinoException if the catalog does not exist and ignoreNotExist
is false
+ * @throws TrinoException if the catalog does not exist and ignoreNotExist
is false, or the
+ * operation fails
*/
public void dropCatalog(
String catalogName, boolean ignoreNotExist, @Nullable String
metalakeArgument) {
@@ -144,10 +144,8 @@ public class DropCatalogStoredProcedure extends
GravitinoStoredProcedure {
GravitinoErrorCode.GRAVITINO_METALAKE_NOT_EXISTS,
"Metalake " + metalake + " not exists.");
} catch (Exception e) {
- throw new TrinoException(
- GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION,
- "Drop catalog failed. " + (StringUtils.isEmpty(e.getMessage()) ? ""
: e.getMessage()),
- e);
+ throw GravitinoErrorCode.toTrinoException(
+ e, "Drop catalog " + NameIdentifier.of(metalake, catalogName) + "
failed");
}
}
}
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnector.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnector.java
index b1d9465ba1..a04c172d88 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnector.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnector.java
@@ -285,8 +285,7 @@ public abstract class TestGravitinoConnector extends
AbstractGravitinoConnectorT
+ "catalog=>'memory1', provider=>'memory', properties =>
Map(array['trino.bypass.unknown-direct-key'], array['10']))",
// The message must carry the reason the registration failed, not just
the fact that it
// did. (?s) lets .* span the newlines of the underlying configuration
error.
- "(?s)Create catalog failed. Create catalog failed due to the loading
process fails\\."
- + ".*unknown-direct-key.*");
+ "(?s)Create catalog failed due to the loading process fails\\." +
".*unknown-direct-key.*");
assertThat(computeActual("show
catalogs").getOnlyColumnAsSet()).doesNotContain("memory1");
// Failed registration still leaves a Gravitino catalog entry visible to
catalog_status /
// system.catalog; drop it so later tests that assume only the seed
"memory" catalog stay
@@ -375,7 +374,7 @@ public abstract class TestGravitinoConnector extends
AbstractGravitinoConnectorT
"call gravitino.system.create_catalog("
+ "catalog=>'memory_failed', provider=>'memory',"
+ " properties => Map(array['trino.bypass.unknown-direct-key'],
array['10']))",
- "(?s)Create catalog failed. Create catalog failed due to the loading
process fails\\..*");
+ "(?s)Create catalog failed due to the loading process fails\\..*");
assertThat(computeActual("show catalogs").getOnlyColumnAsSet())
.doesNotContain("memory_failed");
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
index 82e73441e2..d161d00250 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoConnectorForwardUser.java
@@ -47,6 +47,7 @@ import org.apache.gravitino.NameIdentifier;
import org.apache.gravitino.SupportsSchemas;
import org.apache.gravitino.client.GravitinoAdminClient;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.rel.TableCatalog;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorContext;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorMetadata;
@@ -320,6 +321,29 @@ class TestGravitinoConnectorForwardUser {
TrinoException.class,
() -> connector.resolveSessionMetadata(mockSession("alice",
"token-a")));
assertEquals(GENERIC_INTERNAL_ERROR.toErrorCode(), ex.getErrorCode());
+ assertEquals(
+ "Failed to create per-user Gravitino client for user 'alice':
connection refused",
+ ex.getMessage());
+ }
+
+ @Test
+ void testResolveSessionMetadataMapsLoadMetalakeDenialToPermissionDenied() {
+ CatalogConnectorContext ctx =
+ mockContextWithConfig(
+ ImmutableMap.of(
+ GravitinoAuthProvider.FORWARD_SESSION_USER_KEY, "true",
+ GravitinoAuthProvider.AUTH_TYPE_KEY, "oauth2"));
+ String message = "User alice is not authorized to perform operation
loadMetalake";
+ GravitinoAdminClient client = mock(GravitinoAdminClient.class);
+ when(client.loadMetalake(any())).thenThrow(new
ForbiddenException(message));
+ GravitinoConnector connector = newConnectorWithAuthClient(ctx, session ->
client);
+
+ TrinoException ex =
+ assertThrows(
+ TrinoException.class,
+ () -> connector.resolveSessionMetadata(mockSession("alice",
"token-a")));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), ex.getErrorCode());
+ assertEquals(message, ex.getMessage());
}
/**
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoErrorCode.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoErrorCode.java
new file mode 100644
index 0000000000..0306efb637
--- /dev/null
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoErrorCode.java
@@ -0,0 +1,167 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.trino.connector;
+
+import static io.trino.spi.StandardErrorCode.CATALOG_UNAVAILABLE;
+import static io.trino.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR;
+import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+
+import io.trino.spi.TrinoException;
+import java.io.IOException;
+import java.net.ConnectException;
+import java.util.concurrent.ExecutionException;
+import org.apache.gravitino.exceptions.ConnectionFailedException;
+import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.exceptions.MetalakeNotInUseException;
+import org.apache.gravitino.exceptions.RESTException;
+import org.junit.jupiter.api.Test;
+
+public class TestGravitinoErrorCode {
+
+ @Test
+ public void testTrinoExceptionIsReturnedAsIs() {
+ TrinoException original =
+ new TrinoException(GravitinoErrorCode.GRAVITINO_TABLE_NOT_EXISTS,
"missing");
+ assertSame(original, GravitinoErrorCode.toTrinoException(original));
+ }
+
+ @Test
+ public void testForbiddenKeepsFirstLine() {
+ TrinoException e =
+ GravitinoErrorCode.toTrinoException(
+ new ForbiddenException("denied\n\tat
org.apache.gravitino.Server"));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), e.getErrorCode());
+ assertEquals("denied", e.getMessage());
+ }
+
+ @Test
+ public void testNotInUseIsCatalogUnavailable() {
+ TrinoException e =
+ GravitinoErrorCode.toTrinoException(
+ new MetalakeNotInUseException("Metalake m is not in use"));
+ assertEquals(CATALOG_UNAVAILABLE.toErrorCode(), e.getErrorCode());
+ assertEquals("Metalake m is not in use", e.getMessage());
+ }
+
+ @Test
+ public void testIllegalArgumentIsIllegalArgument() {
+ TrinoException e =
+ GravitinoErrorCode.toTrinoException(new
IllegalArgumentException("Illegal name: 9lives"));
+ assertEquals(GravitinoErrorCode.GRAVITINO_ILLEGAL_ARGUMENT.toErrorCode(),
e.getErrorCode());
+ }
+
+ @Test
+ public void testUnsupportedOperationIsUnsupportedOperation() {
+ TrinoException e =
+ GravitinoErrorCode.toTrinoException(new
UnsupportedOperationException("not supported"));
+ assertEquals(
+ GravitinoErrorCode.GRAVITINO_UNSUPPORTED_OPERATION.toErrorCode(),
e.getErrorCode());
+ }
+
+ @Test
+ public void testSourceConnectionFailureIsConnectionFailed() {
+ TrinoException e =
+ GravitinoErrorCode.toTrinoException(
+ new ConnectionFailedException("Failed to connect to Hive
Metastore"));
+ assertEquals(GravitinoErrorCode.GRAVITINO_CONNECTION_FAILED.toErrorCode(),
e.getErrorCode());
+ assertEquals("Failed to connect to Hive Metastore", e.getMessage());
+ }
+
+ @Test
+ public void testUnreachableServerIsConnectionFailed() {
+ TrinoException e =
+ GravitinoErrorCode.toTrinoException(
+ new RESTException(
+ new ConnectException("Connection refused"),
+ "Failed to connect to Gravitino server"));
+ assertEquals(GravitinoErrorCode.GRAVITINO_CONNECTION_FAILED.toErrorCode(),
e.getErrorCode());
+ }
+
+ @Test
+ public void testUnexpectedServerResponseIsInternalError() {
+ TrinoException e =
+ GravitinoErrorCode.toTrinoException(new RESTException("Unhandled
error: %s", "code 9999"));
+ assertEquals(GENERIC_INTERNAL_ERROR.toErrorCode(), e.getErrorCode());
+
+ e =
+ GravitinoErrorCode.toTrinoException(
+ new RESTException(new IOException("Unexpected token"), "Failed to
parse response"));
+ assertEquals(GENERIC_INTERNAL_ERROR.toErrorCode(), e.getErrorCode());
+ }
+
+ @Test
+ public void testUnclassifiedFailureIsInternalError() {
+ TrinoException e = GravitinoErrorCode.toTrinoException(new
RuntimeException("boom"));
+ assertEquals(GENERIC_INTERNAL_ERROR.toErrorCode(), e.getErrorCode());
+ assertEquals("boom", e.getMessage());
+ }
+
+ @Test
+ public void testExecutionExceptionIsClassifiedByCause() {
+ TrinoException e =
+ GravitinoErrorCode.toTrinoException(
+ new ExecutionException(new ForbiddenException("denied")));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), e.getErrorCode());
+ assertEquals("denied", e.getMessage());
+ }
+
+ @Test
+ public void testContextPrefixesOnlyUnclassifiedFailures() {
+ TrinoException internal =
+ GravitinoErrorCode.toTrinoException(
+ new RuntimeException("boom"), "Drop catalog m.c failed");
+ assertEquals(GENERIC_INTERNAL_ERROR.toErrorCode(),
internal.getErrorCode());
+ assertEquals("Drop catalog m.c failed: boom", internal.getMessage());
+
+ TrinoException denied =
+ GravitinoErrorCode.toTrinoException(
+ new ForbiddenException("denied"), "Drop catalog m.c failed");
+ assertEquals("denied", denied.getMessage());
+ }
+
+ @Test
+ public void testEmptyMessageFallsBackToExceptionName() {
+ TrinoException e = GravitinoErrorCode.toTrinoException(new
IllegalArgumentException(""));
+ assertEquals(GravitinoErrorCode.GRAVITINO_ILLEGAL_ARGUMENT.toErrorCode(),
e.getErrorCode());
+ assertEquals("java.lang.IllegalArgumentException: ", e.getMessage());
+ }
+
+ @Test
+ public void testServerStackTraceIsDroppedFromMessage() {
+ String stack = "\n\tat
org.apache.gravitino.server.web.rest.TableOperations.createTable";
+ TrinoException illegal =
+ GravitinoErrorCode.toTrinoException(
+ new IllegalArgumentException("Illegal name: 9lives" + stack));
+ assertEquals("Illegal name: 9lives", illegal.getMessage());
+
+ TrinoException internal =
+ GravitinoErrorCode.toTrinoException(
+ new RuntimeException("boom" + stack), "Drop catalog m.c failed");
+ assertEquals("Drop catalog m.c failed: boom", internal.getMessage());
+ }
+
+ @Test
+ public void testLeadingBlankLinesAreSkipped() {
+ TrinoException e =
+ GravitinoErrorCode.toTrinoException(new RuntimeException("\n
\nboom\nmore"), "Op failed");
+ assertEquals("Op failed: boom", e.getMessage());
+ }
+}
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/TestCatalogConnectorMetadataErrors.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/TestCatalogConnectorMetadataErrors.java
new file mode 100644
index 0000000000..a5742203df
--- /dev/null
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/catalog/TestCatalogConnectorMetadataErrors.java
@@ -0,0 +1,223 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.trino.connector.catalog;
+
+import static io.trino.spi.StandardErrorCode.CATALOG_UNAVAILABLE;
+import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.anyString;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import io.trino.spi.TrinoException;
+import io.trino.spi.connector.SchemaTableName;
+import org.apache.gravitino.Catalog;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.SupportsSchemas;
+import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.CatalogNotInUseException;
+import org.apache.gravitino.exceptions.ForbiddenException;
+import org.apache.gravitino.exceptions.IllegalNameIdentifierException;
+import org.apache.gravitino.exceptions.NoSuchCatalogException;
+import org.apache.gravitino.exceptions.NoSuchTableException;
+import org.apache.gravitino.exceptions.SchemaAlreadyExistsException;
+import org.apache.gravitino.rel.TableCatalog;
+import org.apache.gravitino.trino.connector.GravitinoErrorCode;
+import org.apache.gravitino.trino.connector.metadata.GravitinoSchema;
+import org.apache.gravitino.trino.connector.metadata.GravitinoTable;
+import org.junit.jupiter.api.Test;
+
+public class TestCatalogConnectorMetadataErrors {
+
+ private static final String DENIED_MESSAGE =
+ "User joel is not authorized to perform operation loadTable on metadata
m.c.s.t";
+
+ @Test
+ public void testLoadCatalogDeniedIsPermissionDenied() {
+ GravitinoMetalake metalake = mock(GravitinoMetalake.class);
+ when(metalake.loadCatalog(anyString())).thenThrow(new
ForbiddenException(DENIED_MESSAGE));
+
+ TrinoException e =
+ assertThrows(
+ TrinoException.class,
+ () -> new CatalogConnectorMetadata(metalake,
NameIdentifier.of("m", "c")));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), e.getErrorCode());
+ assertEquals(DENIED_MESSAGE, e.getMessage());
+ }
+
+ @Test
+ public void testGetTableDeniedIsPermissionDenied() {
+ TableCatalog tableCatalog = mock(TableCatalog.class);
+ when(tableCatalog.loadTable(any())).thenThrow(new
ForbiddenException(DENIED_MESSAGE));
+ CatalogConnectorMetadata metadata = createMetadata(tableCatalog);
+
+ TrinoException e = assertThrows(TrinoException.class, () ->
metadata.getTable("s", "t"));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), e.getErrorCode());
+ assertEquals(DENIED_MESSAGE, e.getMessage());
+ }
+
+ @Test
+ public void testDeniedMessageWithServerStackKeepsFirstLine() {
+ // The REST client appends the server stack trace to the message of a
refusal.
+ String messageWithStack =
+ DENIED_MESSAGE
+ + "\n"
+ + "org.apache.gravitino.exceptions.ForbiddenException: "
+ + DENIED_MESSAGE
+ + "\n"
+ + "\tat
org.apache.gravitino.server.web.rest.TableOperations.loadTable(TableOperations.java:1)";
+ TableCatalog tableCatalog = mock(TableCatalog.class);
+ when(tableCatalog.loadTable(any())).thenThrow(new
ForbiddenException(messageWithStack));
+ CatalogConnectorMetadata metadata = createMetadata(tableCatalog);
+
+ TrinoException e = assertThrows(TrinoException.class, () ->
metadata.getTable("s", "t"));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), e.getErrorCode());
+ assertEquals(DENIED_MESSAGE, e.getMessage());
+ }
+
+ @Test
+ public void testIllegalArgumentWithoutMessageIsIllegalArgument() {
+ TableCatalog tableCatalog = mock(TableCatalog.class);
+ when(tableCatalog.loadTable(any())).thenThrow(new
IllegalNameIdentifierException());
+ CatalogConnectorMetadata metadata = createMetadata(tableCatalog);
+
+ TrinoException e = assertThrows(TrinoException.class, () ->
metadata.getTable("s", "t"));
+ assertEquals(GravitinoErrorCode.GRAVITINO_ILLEGAL_ARGUMENT.toErrorCode(),
e.getErrorCode());
+ }
+
+ @Test
+ public void testGetMissingTableKeepsTableNotExists() {
+ TableCatalog tableCatalog = mock(TableCatalog.class);
+ when(tableCatalog.loadTable(any())).thenThrow(new
NoSuchTableException("missing"));
+ CatalogConnectorMetadata metadata = createMetadata(tableCatalog);
+
+ TrinoException e = assertThrows(TrinoException.class, () ->
metadata.getTable("s", "t"));
+ assertEquals(GravitinoErrorCode.GRAVITINO_TABLE_NOT_EXISTS.toErrorCode(),
e.getErrorCode());
+ }
+
+ @Test
+ public void testTableExistsDeniedIsPermissionDenied() {
+ TableCatalog tableCatalog = mock(TableCatalog.class);
+ when(tableCatalog.tableExists(any())).thenThrow(new
ForbiddenException(DENIED_MESSAGE));
+ CatalogConnectorMetadata metadata = createMetadata(tableCatalog);
+
+ TrinoException e = assertThrows(TrinoException.class, () ->
metadata.tableExists("s", "t"));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), e.getErrorCode());
+ }
+
+ @Test
+ public void testCreateTableWithIllegalNameIsIllegalArgument() {
+ String message = "The TABLE name 9lives is illegal. Illegal name: 9lives";
+ TableCatalog tableCatalog = mock(TableCatalog.class);
+ when(tableCatalog.createTable(any(), any(), any(), any(), any(), any(),
any(), any()))
+ .thenThrow(new IllegalArgumentException(message));
+ CatalogConnectorMetadata metadata = createMetadata(tableCatalog);
+ GravitinoTable table = mock(GravitinoTable.class);
+ when(table.getSchemaName()).thenReturn("s");
+ when(table.getName()).thenReturn("9lives");
+
+ TrinoException e = assertThrows(TrinoException.class, () ->
metadata.createTable(table, false));
+ assertEquals(GravitinoErrorCode.GRAVITINO_ILLEGAL_ARGUMENT.toErrorCode(),
e.getErrorCode());
+ assertEquals(message, e.getMessage());
+ }
+
+ @Test
+ public void testDropTableDeniedIsPermissionDenied() {
+ TableCatalog tableCatalog = mock(TableCatalog.class);
+ when(tableCatalog.dropTable(any())).thenThrow(new
ForbiddenException(DENIED_MESSAGE));
+ CatalogConnectorMetadata metadata = createMetadata(tableCatalog);
+
+ TrinoException e =
+ assertThrows(TrinoException.class, () -> metadata.dropTable(new
SchemaTableName("s", "t")));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), e.getErrorCode());
+ }
+
+ @Test
+ public void testAlterTableDeniedIsPermissionDenied() {
+ TableCatalog tableCatalog = mock(TableCatalog.class);
+ when(tableCatalog.alterTable(any(), any())).thenThrow(new
ForbiddenException(DENIED_MESSAGE));
+ CatalogConnectorMetadata metadata = createMetadata(tableCatalog);
+
+ TrinoException e =
+ assertThrows(
+ TrinoException.class,
+ () ->
+ metadata.renameTable(
+ new SchemaTableName("s", "t"), new SchemaTableName("s",
"t2")));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), e.getErrorCode());
+ }
+
+ @Test
+ public void testLoadCatalogNotInUseIsCatalogUnavailable() {
+ String message = "Catalog m.c is not in use";
+ GravitinoMetalake metalake = mock(GravitinoMetalake.class);
+ when(metalake.loadCatalog(anyString())).thenThrow(new
CatalogNotInUseException(message));
+
+ TrinoException e =
+ assertThrows(
+ TrinoException.class,
+ () -> new CatalogConnectorMetadata(metalake,
NameIdentifier.of("m", "c")));
+ assertEquals(CATALOG_UNAVAILABLE.toErrorCode(), e.getErrorCode());
+ assertEquals(message, e.getMessage());
+ }
+
+ @Test
+ public void testCreateExistingSchemaIsSchemaAlreadyExists() {
+ SupportsSchemas schemas = mock(SupportsSchemas.class);
+ when(schemas.createSchema(any(), any(), any()))
+ .thenThrow(new SchemaAlreadyExistsException("exists"));
+ CatalogConnectorMetadata metadata = createMetadata(schemas,
mock(TableCatalog.class));
+
+ TrinoException e =
+ assertThrows(
+ TrinoException.class, () ->
metadata.createSchema(mock(GravitinoSchema.class)));
+ assertEquals(
+ GravitinoErrorCode.GRAVITINO_SCHEMA_ALREADY_EXISTS.toErrorCode(),
e.getErrorCode());
+ }
+
+ @Test
+ public void testCreateSchemaInMissingCatalogIsCatalogNotExists() {
+ SupportsSchemas schemas = mock(SupportsSchemas.class);
+ when(schemas.createSchema(any(), any(), any()))
+ .thenThrow(new NoSuchCatalogException("missing"));
+ CatalogConnectorMetadata metadata = createMetadata(schemas,
mock(TableCatalog.class));
+
+ TrinoException e =
+ assertThrows(
+ TrinoException.class, () ->
metadata.createSchema(mock(GravitinoSchema.class)));
+
assertEquals(GravitinoErrorCode.GRAVITINO_CATALOG_NOT_EXISTS.toErrorCode(),
e.getErrorCode());
+ }
+
+ private static CatalogConnectorMetadata createMetadata(TableCatalog
tableCatalog) {
+ return createMetadata(mock(SupportsSchemas.class), tableCatalog);
+ }
+
+ private static CatalogConnectorMetadata createMetadata(
+ SupportsSchemas schemas, TableCatalog tableCatalog) {
+ GravitinoMetalake metalake = mock(GravitinoMetalake.class);
+ Catalog catalog = mock(Catalog.class);
+ when(metalake.loadCatalog(anyString())).thenReturn(catalog);
+ when(catalog.asSchemas()).thenReturn(schemas);
+ when(catalog.asTableCatalog()).thenReturn(tableCatalog);
+ when(catalog.asFunctionCatalog()).thenThrow(new
UnsupportedOperationException("unsupported"));
+ return new CatalogConnectorMetadata(metalake, NameIdentifier.of("m", "c"));
+ }
+}
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestAlterCatalogStoredProcedure.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestAlterCatalogStoredProcedure.java
index 5ec6fe7133..72f8e4347d 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestAlterCatalogStoredProcedure.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestAlterCatalogStoredProcedure.java
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.trino.connector.system.storedprocedure;
+import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -35,6 +36,7 @@ import com.google.common.collect.ImmutableMap;
import io.trino.spi.TrinoException;
import java.util.List;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.trino.connector.GravitinoErrorCode;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorContext;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorManager;
@@ -102,7 +104,31 @@ public class TestAlterCatalogStoredProcedure {
assertThrows(
TrinoException.class,
() -> procedure.alterCatalog(CATALOG, ImmutableMap.of("k", "v"),
List.of(), null));
+ assertEquals(
+ GravitinoErrorCode.GRAVITINO_CATALOG_NOT_EXISTS.toErrorCode(),
error.getErrorCode());
assertTrue(error.getMessage().contains("already registered"),
error.getMessage());
verify(otherMetalake, never()).alterCatalog(anyString(), any());
}
+
+ @Test
+ public void testAlterCatalogDeniedIsPermissionDenied() {
+ String message = "User joel is not authorized to perform operation
alterCatalog";
+ CatalogConnectorManager manager = mock(CatalogConnectorManager.class);
+ CatalogConnectorContext context = mock(CatalogConnectorContext.class);
+ GravitinoMetalake metalake = mock(GravitinoMetalake.class);
+ when(manager.getCatalogConnector(METALAKE, CATALOG)).thenReturn(context);
+ when(context.getMetalake()).thenReturn(metalake);
+ when(context.getCatalog())
+ .thenReturn(new GravitinoCatalog(METALAKE, "memory", CATALOG,
ImmutableMap.of(), 1L));
+ when(metalake.alterCatalog(eq(CATALOG), any())).thenThrow(new
ForbiddenException(message));
+
+ AlterCatalogStoredProcedure procedure = new
AlterCatalogStoredProcedure(manager, METALAKE);
+
+ TrinoException error =
+ assertThrows(
+ TrinoException.class,
+ () -> procedure.alterCatalog(CATALOG, ImmutableMap.of("k", "v"),
List.of(), null));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), error.getErrorCode());
+ assertEquals(message, error.getMessage());
+ }
}
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestCreateCatalogStoredProcedure.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestCreateCatalogStoredProcedure.java
index 9a451f386a..16b61aa667 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestCreateCatalogStoredProcedure.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestCreateCatalogStoredProcedure.java
@@ -18,13 +18,17 @@
*/
package org.apache.gravitino.trino.connector.system.storedprocedure;
+import static io.trino.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR;
+import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertThrows;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.mockito.ArgumentMatchers.any;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
import static org.mockito.Mockito.times;
@@ -34,8 +38,10 @@ import static org.mockito.Mockito.when;
import com.google.common.collect.ImmutableMap;
import io.trino.spi.TrinoException;
import java.util.Map;
+import java.util.concurrent.TimeoutException;
import org.apache.gravitino.Catalog;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.trino.connector.GravitinoErrorCode;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorContext;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorManager;
@@ -103,6 +109,85 @@ public class TestCreateCatalogStoredProcedure {
TrinoException.class,
() -> procedure.createCatalog(CATALOG, "memory", PROPERTIES,
false, null));
verify(requested, times(1)).createCatalog(eq(CATALOG), any(),
eq("memory"), any(), any());
+ assertEquals(GravitinoErrorCode.GRAVITINO_OPERATION_FAILED.toErrorCode(),
error.getErrorCode());
assertTrue(error.getMessage().contains("already registered"),
error.getMessage());
}
+
+ @Test
+ public void testCreateCatalogDeniedIsPermissionDenied() {
+ String message = "User joel is not authorized to perform operation
createCatalog";
+ CatalogConnectorManager manager = mock(CatalogConnectorManager.class);
+ GravitinoMetalake metalake = mock(GravitinoMetalake.class);
+ when(manager.getMetalake(METALAKE)).thenReturn(metalake);
+ when(metalake.createCatalog(anyString(), any(), anyString(), anyString(),
any()))
+ .thenThrow(new ForbiddenException(message));
+
+ CreateCatalogStoredProcedure procedure = new
CreateCatalogStoredProcedure(manager, METALAKE);
+
+ TrinoException error =
+ assertThrows(
+ TrinoException.class,
+ () -> procedure.createCatalog(CATALOG, "memory", PROPERTIES,
false, null));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), error.getErrorCode());
+ assertEquals(message, error.getMessage());
+ }
+
+ @Test
+ public void testCreateCatalogIllegalArgumentIsIllegalArgument() {
+ CatalogConnectorManager manager = mock(CatalogConnectorManager.class);
+ GravitinoMetalake metalake = mock(GravitinoMetalake.class);
+ when(manager.getMetalake(METALAKE)).thenReturn(metalake);
+ when(metalake.createCatalog(anyString(), any(), anyString(), anyString(),
any()))
+ .thenThrow(new IllegalArgumentException("Unsupported provider: bad"));
+
+ CreateCatalogStoredProcedure procedure = new
CreateCatalogStoredProcedure(manager, METALAKE);
+
+ TrinoException error =
+ assertThrows(
+ TrinoException.class,
+ () -> procedure.createCatalog(CATALOG, "bad", PROPERTIES, false,
null));
+ assertEquals(GravitinoErrorCode.GRAVITINO_ILLEGAL_ARGUMENT.toErrorCode(),
error.getErrorCode());
+ assertTrue(error.getMessage().contains("Unsupported provider: bad"),
error.getMessage());
+ }
+
+ @Test
+ public void
testCreateCatalogUnclassifiedFailureKeepsContextAndRedactsSecrets() throws
Exception {
+ CatalogConnectorManager manager = mock(CatalogConnectorManager.class);
+
when(manager.getMetalake(METALAKE)).thenReturn(mock(GravitinoMetalake.class));
+ doThrow(
+ new RuntimeException(
+ "CREATE CATALOG c USING jdbc WITH
(\"trino.jdbc.password\"='s3cret') failed"))
+ .when(manager)
+ .loadMetalakeSync();
+
+ CreateCatalogStoredProcedure procedure = new
CreateCatalogStoredProcedure(manager, METALAKE);
+
+ TrinoException error =
+ assertThrows(
+ TrinoException.class,
+ () -> procedure.createCatalog(CATALOG, "memory", PROPERTIES,
false, null));
+ assertEquals(GENERIC_INTERNAL_ERROR.toErrorCode(), error.getErrorCode());
+ assertTrue(
+ error.getMessage().startsWith("Create catalog
test_metalake.test_catalog failed: "),
+ error.getMessage());
+ assertFalse(error.getMessage().contains("s3cret"), error.getMessage());
+ }
+
+ @Test
+ public void testCreateCatalogReloadTimeoutKeepsContext() throws Exception {
+ CatalogConnectorManager manager = mock(CatalogConnectorManager.class);
+
when(manager.getMetalake(METALAKE)).thenReturn(mock(GravitinoMetalake.class));
+ doThrow(new TimeoutException()).when(manager).loadMetalakeSync();
+
+ CreateCatalogStoredProcedure procedure = new
CreateCatalogStoredProcedure(manager, METALAKE);
+
+ TrinoException error =
+ assertThrows(
+ TrinoException.class,
+ () -> procedure.createCatalog(CATALOG, "memory", PROPERTIES,
false, null));
+ assertEquals(GENERIC_INTERNAL_ERROR.toErrorCode(), error.getErrorCode());
+ assertEquals(
+ "Create catalog test_metalake.test_catalog failed:
java.util.concurrent.TimeoutException",
+ error.getMessage());
+ }
}
diff --git
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestDropCatalogStoredProcedure.java
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestDropCatalogStoredProcedure.java
index cffd2d5a9f..72310271f8 100644
---
a/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestDropCatalogStoredProcedure.java
+++
b/trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/system/storedprocedure/TestDropCatalogStoredProcedure.java
@@ -18,6 +18,7 @@
*/
package org.apache.gravitino.trino.connector.system.storedprocedure;
+import static io.trino.spi.StandardErrorCode.PERMISSION_DENIED;
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -33,6 +34,7 @@ import static org.mockito.Mockito.when;
import io.trino.spi.TrinoException;
import org.apache.gravitino.client.GravitinoMetalake;
+import org.apache.gravitino.exceptions.ForbiddenException;
import org.apache.gravitino.trino.connector.GravitinoErrorCode;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorContext;
import org.apache.gravitino.trino.connector.catalog.CatalogConnectorManager;
@@ -135,22 +137,13 @@ public class TestDropCatalogStoredProcedure {
DropCatalogStoredProcedure procedure = new
DropCatalogStoredProcedure(manager, METALAKE);
- // The internally-thrown "not exists" TrinoException is caught by the
outer catch(Exception)
- // and re-wrapped as GRAVITINO_UNSUPPORTED_OPERATION, with the original
exception preserved
- // as the cause. This matches the historical behavior observed in the
issue's stack trace.
- // The contract we verify is: the user-visible message still tells them
the catalog does not
- // exist, and the root cause carries the GRAVITINO_CATALOG_NOT_EXISTS
error code.
TrinoException error =
assertThrows(TrinoException.class, () ->
procedure.dropCatalog(CATALOG, false, null));
assertTrue(
error.getMessage().contains("not exists"),
() -> "Expected message to contain 'not exists' but was: " +
error.getMessage());
- Throwable cause = error.getCause();
- assertTrue(
- cause instanceof TrinoException, () -> "Expected TrinoException cause,
was: " + cause);
assertEquals(
- GravitinoErrorCode.GRAVITINO_CATALOG_NOT_EXISTS.toErrorCode(),
- ((TrinoException) cause).getErrorCode());
+ GravitinoErrorCode.GRAVITINO_CATALOG_NOT_EXISTS.toErrorCode(),
error.getErrorCode());
}
@Test
@@ -173,4 +166,20 @@ public class TestDropCatalogStoredProcedure {
// Local-cache reload should NOT be triggered when the local connector was
already absent.
verify(manager, never()).catalogConnectorExist(anyString());
}
+
+ @Test
+ public void testDropCatalogDeniedIsPermissionDenied() {
+ String message = "User joel is not authorized to perform operation
dropCatalog";
+ CatalogConnectorManager manager = mock(CatalogConnectorManager.class);
+ GravitinoMetalake metalake = mock(GravitinoMetalake.class);
+ when(manager.getMetalake(METALAKE)).thenReturn(metalake);
+ when(metalake.dropCatalog(CATALOG, true)).thenThrow(new
ForbiddenException(message));
+
+ DropCatalogStoredProcedure procedure = new
DropCatalogStoredProcedure(manager, METALAKE);
+
+ TrinoException error =
+ assertThrows(TrinoException.class, () ->
procedure.dropCatalog(CATALOG, false, null));
+ assertEquals(PERMISSION_DENIED.toErrorCode(), error.getErrorCode());
+ assertEquals(message, error.getMessage());
+ }
}
