Copilot commented on code in PR #10494:
URL: https://github.com/apache/gravitino/pull/10494#discussion_r3008305243
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -32,10 +33,13 @@
import org.apache.gravitino.SupportsSchemas;
import org.apache.gravitino.client.GravitinoMetalake;
import org.apache.gravitino.exceptions.NoSuchCatalogException;
+import org.apache.gravitino.exceptions.NoSuchFunctionException;
Review Comment:
The import for `NoSuchFunctionException` is unused (it’s only referenced in
JavaDoc). Spotless is configured with `removeUnusedImports()`, so this will
fail formatting checks unless removed or referenced in code/JavaDoc using FQN
without importing.
```suggestion
```
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoMetadata.java:
##########
@@ -679,4 +695,85 @@ private String getColumnName(
}
return internalMetadataColumnMetadata.getName();
}
+
+ @Override
+ public Collection<LanguageFunction> listLanguageFunctions(
+ ConnectorSession session, String schemaName) {
+ if (!catalogConnectorMetadata.supportsFunctions()) {
+ return List.of();
+ }
+ return
Arrays.stream(catalogConnectorMetadata.listFunctionInfos(schemaName))
+ .flatMap(function -> toLanguageFunctions(function).stream())
+ .toList();
+ }
+
+ @Override
+ public Collection<LanguageFunction> getLanguageFunctions(
+ ConnectorSession session, SchemaFunctionName name) {
+ if (!catalogConnectorMetadata.supportsFunctions()) {
+ return List.of();
+ }
+ try {
+ Function function =
+ catalogConnectorMetadata.getFunction(name.getSchemaName(),
name.getFunctionName());
+ if (function == null) {
+ return List.of();
+ }
+ return toLanguageFunctions(function);
+ } catch (NoSuchFunctionException e) {
+ LOG.debug("Function {} not found in schema {}", name.getFunctionName(),
name.getSchemaName());
+ return List.of();
+ }
+ }
+
+ /**
+ * Converts a Gravitino function to a collection of Trino LanguageFunction
instances. Only SQL
+ * implementations with TRINO runtime are included. Each definition with a
Trino SQL
+ * implementation produces one LanguageFunction. The signature token is
generated from the
+ * function name and parameter types.
+ */
+ private Collection<LanguageFunction> toLanguageFunctions(Function function) {
+ List<LanguageFunction> result = new ArrayList<>();
+ for (FunctionDefinition definition : function.definitions()) {
+ for (FunctionImpl impl : definition.impls()) {
+ if (!isTrinoSqlImplementation(impl)) {
+ continue;
+ }
+ String sql = ((SQLImpl) impl).sql();
+ try {
+ String signatureToken = buildSignatureToken(function.name(),
definition.parameters());
+ result.add(new LanguageFunction(signatureToken, sql, List.of(),
Optional.empty()));
+ } catch (Exception e) {
+ LOG.warn(
+ "Failed to build signature token for function {}: {}",
+ function.name(),
+ e.getMessage());
Review Comment:
`toLanguageFunctions` catches a generic `Exception` and logs only
`e.getMessage()`, which can hide the root cause and stack trace (and the repo
guidelines discourage `catch (Exception e)`). Catch the specific expected
exceptions (e.g., unsupported type conversion) and include the exception in the
log call so failures are diagnosable.
```suggestion
} catch (IllegalArgumentException | UnsupportedOperationException e)
{
LOG.warn(
"Failed to build signature token for function {}",
function.name(),
e);
```
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -396,4 +412,46 @@ public void setColumnType(SchemaTableName schemaTableName,
String columnName, Ty
String[] columnNames = {columnName};
applyAlter(schemaTableName, TableChange.updateColumnType(columnNames,
type));
}
+
+ /**
+ * Checks whether the catalog supports function operations.
+ *
+ * @return true if the catalog supports function operations, false otherwise
+ */
+ public boolean supportsFunctions() {
+ return functionCatalog != null;
+ }
+
+ /**
+ * Lists all functions with details in the specified schema.
+ *
+ * @param schemaName the name of the schema
+ * @return an array of functions, or an empty array if functions are not
supported
+ */
+ public Function[] listFunctionInfos(String schemaName) {
+ if (!supportsFunctions()) {
+ throw new UnsupportedOperationException("Catalog does not support
functions");
+ }
Review Comment:
JavaDoc for `listFunctionInfos` says it returns an empty array when
functions are not supported, but the method currently throws
`UnsupportedOperationException` in that case. Please align the JavaDoc with the
actual behavior (or change the method to match the documented contract).
--
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]
