Copilot commented on code in PR #10494:
URL: https://github.com/apache/gravitino/pull/10494#discussion_r2974114580
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoMetadata.java:
##########
@@ -679,4 +692,76 @@ private String getColumnName(
}
return internalMetadataColumnMetadata.getName();
}
+
+ @Override
+ public Collection<LanguageFunction> listLanguageFunctions(
+ ConnectorSession session, String schemaName) {
+ if (!catalogConnectorMetadata.supportsFunctions()) {
+ return List.of();
+ }
+ Function[] functions =
catalogConnectorMetadata.listFunctionInfos(schemaName);
+ List<LanguageFunction> result = new ArrayList<>();
+ for (Function function : functions) {
+ result.addAll(toLanguageFunctions(function));
+ }
+ LOG.debug("Listed {} language functions in schema {}", result.size(),
schemaName);
+ return result;
+ }
+
+ @Override
+ public Collection<LanguageFunction> getLanguageFunctions(
+ ConnectorSession session, SchemaFunctionName name) {
+ if (!catalogConnectorMetadata.supportsFunctions()) {
+ return List.of();
+ }
+ Function function =
+ catalogConnectorMetadata.getFunction(name.getSchemaName(),
name.getFunctionName());
+ if (function == null) {
+ return List.of();
+ }
+ return toLanguageFunctions(function);
+ }
+
+ /**
+ * 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();
+ String signatureToken = buildSignatureToken(function.name(),
definition.parameters());
+ result.add(new LanguageFunction(signatureToken, sql, List.of(),
Optional.empty()));
+ }
+ }
+ return result;
+ }
+
+ private boolean isTrinoSqlImplementation(FunctionImpl impl) {
+ return FunctionImpl.RuntimeType.TRINO.equals(impl.runtime())
+ && FunctionImpl.Language.SQL.equals(impl.language());
+ }
+
+ /**
+ * Builds a signature token from function name and parameters. The token is
lowercase as required
+ * by Trino's LanguageFunction.
+ */
+ private String buildSignatureToken(String functionName, FunctionParam[]
params) {
+ StringBuilder sb = new StringBuilder(functionName.toLowerCase());
+ sb.append("(");
+ for (int i = 0; i < params.length; i++) {
+ if (i > 0) {
+ sb.append(",");
+ }
+ sb.append(params[i].dataType().simpleString().toLowerCase());
Review Comment:
`buildSignatureToken` uses Gravitino type `simpleString()` values (e.g.,
`string`) as the argument type tokens. Trino type names/signatures differ
(e.g., Gravitino STRING is mapped to Trino VARCHAR via
`GeneralDataTypeTransformer`), so this will generate signature tokens that
Trino cannot match for some types. Consider generating tokens from the Trino
`Type` returned by `metadataAdapter.getDataTypeTransformer().getTrinoType(...)`
(e.g., `getDisplayName()` / type signature), rather than from Gravitino
`simpleString()`.
```suggestion
Type trinoType =
metadataAdapter.getDataTypeTransformer().getTrinoType(params[i].dataType());
String typeToken;
if (trinoType != null) {
typeToken = trinoType.getDisplayName().toLowerCase();
} else {
typeToken = params[i].dataType().simpleString().toLowerCase();
}
sb.append(typeToken);
```
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/GravitinoMetadata.java:
##########
@@ -679,4 +692,76 @@ private String getColumnName(
}
return internalMetadataColumnMetadata.getName();
}
+
+ @Override
+ public Collection<LanguageFunction> listLanguageFunctions(
+ ConnectorSession session, String schemaName) {
+ if (!catalogConnectorMetadata.supportsFunctions()) {
+ return List.of();
+ }
+ Function[] functions =
catalogConnectorMetadata.listFunctionInfos(schemaName);
+ List<LanguageFunction> result = new ArrayList<>();
+ for (Function function : functions) {
+ result.addAll(toLanguageFunctions(function));
+ }
+ LOG.debug("Listed {} language functions in schema {}", result.size(),
schemaName);
+ return result;
+ }
+
+ @Override
+ public Collection<LanguageFunction> getLanguageFunctions(
+ ConnectorSession session, SchemaFunctionName name) {
+ if (!catalogConnectorMetadata.supportsFunctions()) {
+ return List.of();
+ }
+ Function function =
+ catalogConnectorMetadata.getFunction(name.getSchemaName(),
name.getFunctionName());
+ if (function == null) {
+ return List.of();
+ }
+ return toLanguageFunctions(function);
+ }
+
+ /**
+ * 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();
+ String signatureToken = buildSignatureToken(function.name(),
definition.parameters());
+ result.add(new LanguageFunction(signatureToken, sql, List.of(),
Optional.empty()));
+ }
+ }
+ return result;
+ }
+
+ private boolean isTrinoSqlImplementation(FunctionImpl impl) {
+ return FunctionImpl.RuntimeType.TRINO.equals(impl.runtime())
+ && FunctionImpl.Language.SQL.equals(impl.language());
+ }
+
+ /**
+ * Builds a signature token from function name and parameters. The token is
lowercase as required
+ * by Trino's LanguageFunction.
+ */
+ private String buildSignatureToken(String functionName, FunctionParam[]
params) {
+ StringBuilder sb = new StringBuilder(functionName.toLowerCase());
+ sb.append("(");
+ for (int i = 0; i < params.length; i++) {
+ if (i > 0) {
+ sb.append(",");
+ }
+ sb.append(params[i].dataType().simpleString().toLowerCase());
+ }
Review Comment:
`toLowerCase()` here is locale-sensitive. To avoid surprising behavior under
non-English default locales (e.g., Turkish locale), use
`toLowerCase(Locale.ENGLISH)` (and add the `Locale` import) when generating the
signature token.
##########
trino-connector/trino-connector/src/main/java/org/apache/gravitino/trino/connector/catalog/CatalogConnectorMetadata.java:
##########
@@ -396,4 +411,50 @@ 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 (functionCatalog == null) {
+ return new Function[0];
+ }
+ try {
+ return functionCatalog.listFunctionInfos(Namespace.of(schemaName));
+ } catch (NoSuchSchemaException e) {
+ throw new TrinoException(
+ GravitinoErrorCode.GRAVITINO_SCHEMA_NOT_EXISTS,
SCHEMA_DOES_NOT_EXIST_MSG, e);
+ }
+ }
+
+ /**
+ * Retrieves a function by its schema and function name.
+ *
+ * @param schemaName the name of the schema
+ * @param functionName the name of the function
+ * @return the function, or null if functions are not supported or the
function does not exist
+ */
+ @Nullable
+ public Function getFunction(String schemaName, String functionName) {
+ if (functionCatalog == null) {
+ return null;
+ }
+ try {
+ return functionCatalog.getFunction(NameIdentifier.of(schemaName,
functionName));
+ } catch (org.apache.gravitino.exceptions.NoSuchFunctionException e) {
+ return null;
Review Comment:
`getFunction` catches
`org.apache.gravitino.exceptions.NoSuchFunctionException` using a fully
qualified class name. Please add a normal import and use the simple class name
to match the project's Java import/style guidance and keep the code
consistent/readable.
##########
trino-connector/trino-connector/src/test/java/org/apache/gravitino/trino/connector/TestGravitinoMetadataFunction.java:
##########
@@ -0,0 +1,248 @@
+/*
+ * 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 org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import io.trino.spi.connector.ConnectorMetadata;
+import io.trino.spi.connector.ConnectorSession;
+import io.trino.spi.function.LanguageFunction;
+import io.trino.spi.function.SchemaFunctionName;
+import java.util.Collection;
+import java.util.List;
+import org.apache.gravitino.Audit;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionImpl;
+import org.apache.gravitino.function.FunctionImpls;
+import org.apache.gravitino.function.FunctionParam;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.rel.types.Types;
+import org.apache.gravitino.trino.connector.catalog.CatalogConnectorMetadata;
+import
org.apache.gravitino.trino.connector.catalog.CatalogConnectorMetadataAdapter;
+import org.junit.jupiter.api.Test;
+
+public class TestGravitinoMetadataFunction {
+
+ @Test
+ public void testListLanguageFunctionsReturnsTrinoSqlFunctions() {
+ Function function =
+ createMockFunction("my_func", "RETURN x + 1",
FunctionImpl.RuntimeType.TRINO);
+ CatalogConnectorMetadata catalogMetadata =
mock(CatalogConnectorMetadata.class);
+ when(catalogMetadata.supportsFunctions()).thenReturn(true);
+ when(catalogMetadata.listFunctionInfos("test_schema")).thenReturn(new
Function[] {function});
+
+ GravitinoMetadata metadata = createTestMetadata(catalogMetadata);
+ ConnectorSession session = mock(ConnectorSession.class);
+
+ Collection<LanguageFunction> functions =
metadata.listLanguageFunctions(session, "test_schema");
+ assertEquals(1, functions.size());
+
+ LanguageFunction langFunc = functions.iterator().next();
+ assertEquals("RETURN x + 1", langFunc.sql());
+ assertEquals("my_func(integer)", langFunc.signatureToken());
+ }
+
+ @Test
+ public void testGetLanguageFunctionsReturnsTrinoSqlFunctions() {
+ Function function =
+ createMockFunction("my_func", "RETURN x + 1",
FunctionImpl.RuntimeType.TRINO);
+ CatalogConnectorMetadata catalogMetadata =
mock(CatalogConnectorMetadata.class);
+ when(catalogMetadata.supportsFunctions()).thenReturn(true);
+ when(catalogMetadata.getFunction("test_schema",
"my_func")).thenReturn(function);
+
+ GravitinoMetadata metadata = createTestMetadata(catalogMetadata);
+ ConnectorSession session = mock(ConnectorSession.class);
+
+ Collection<LanguageFunction> functions =
+ metadata.getLanguageFunctions(session, new
SchemaFunctionName("test_schema", "my_func"));
+ assertEquals(1, functions.size());
+
+ LanguageFunction langFunc = functions.iterator().next();
+ assertEquals("RETURN x + 1", langFunc.sql());
+ }
+
+ @Test
+ public void testListLanguageFunctionsFiltersNonTrinoRuntime() {
+ Function sparkFunction =
+ createMockFunction("spark_func", "RETURN 1",
FunctionImpl.RuntimeType.SPARK);
+ Function trinoFunction =
+ createMockFunction("trino_func", "RETURN 2",
FunctionImpl.RuntimeType.TRINO);
+
+ CatalogConnectorMetadata catalogMetadata =
mock(CatalogConnectorMetadata.class);
+ when(catalogMetadata.supportsFunctions()).thenReturn(true);
+ when(catalogMetadata.listFunctionInfos("test_schema"))
+ .thenReturn(new Function[] {sparkFunction, trinoFunction});
+
+ GravitinoMetadata metadata = createTestMetadata(catalogMetadata);
+ ConnectorSession session = mock(ConnectorSession.class);
+
+ Collection<LanguageFunction> functions =
metadata.listLanguageFunctions(session, "test_schema");
+ assertEquals(1, functions.size());
+ assertEquals("RETURN 2", functions.iterator().next().sql());
+ }
+
+ @Test
+ public void testListLanguageFunctionsWhenUnsupported() {
+ CatalogConnectorMetadata catalogMetadata =
mock(CatalogConnectorMetadata.class);
+ when(catalogMetadata.supportsFunctions()).thenReturn(false);
+
+ GravitinoMetadata metadata = createTestMetadata(catalogMetadata);
+ ConnectorSession session = mock(ConnectorSession.class);
+
+ Collection<LanguageFunction> functions =
metadata.listLanguageFunctions(session, "test_schema");
+ assertTrue(functions.isEmpty());
+ }
+
+ @Test
+ public void testGetLanguageFunctionsWhenFunctionNotFound() {
+ CatalogConnectorMetadata catalogMetadata =
mock(CatalogConnectorMetadata.class);
+ when(catalogMetadata.supportsFunctions()).thenReturn(true);
+ when(catalogMetadata.getFunction("test_schema",
"no_such_func")).thenReturn(null);
+
+ GravitinoMetadata metadata = createTestMetadata(catalogMetadata);
+ ConnectorSession session = mock(ConnectorSession.class);
+
+ Collection<LanguageFunction> functions =
+ metadata.getLanguageFunctions(
+ session, new SchemaFunctionName("test_schema", "no_such_func"));
+ assertTrue(functions.isEmpty());
+ }
+
+ @Test
+ public void testMultipleDefinitionsAndImpls() {
+ FunctionParam param1 = createMockParam("x", Types.IntegerType.get());
+ FunctionParam param2 = createMockParam("x", Types.StringType.get());
+
+ FunctionImpl trinoSqlImpl1 =
+ FunctionImpls.ofSql(FunctionImpl.RuntimeType.TRINO, "RETURN x + 1");
+ FunctionImpl trinoSqlImpl2 =
+ FunctionImpls.ofSql(FunctionImpl.RuntimeType.TRINO, "RETURN
length(x)");
+ FunctionImpl sparkImpl =
FunctionImpls.ofSql(FunctionImpl.RuntimeType.SPARK, "RETURN x * 2");
+
+ FunctionDefinition def1 = createMockDefinition(new FunctionParam[]
{param1}, trinoSqlImpl1);
+ FunctionDefinition def2 =
+ createMockDefinition(new FunctionParam[] {param2}, trinoSqlImpl2,
sparkImpl);
+
+ Function function = createMockFunctionWithDefinitions("multi_func", def1,
def2);
+
+ CatalogConnectorMetadata catalogMetadata =
mock(CatalogConnectorMetadata.class);
+ when(catalogMetadata.supportsFunctions()).thenReturn(true);
+ when(catalogMetadata.listFunctionInfos("test_schema")).thenReturn(new
Function[] {function});
+
+ GravitinoMetadata metadata = createTestMetadata(catalogMetadata);
+ ConnectorSession session = mock(ConnectorSession.class);
+
+ Collection<LanguageFunction> functions =
metadata.listLanguageFunctions(session, "test_schema");
+ // Should have 2 Trino SQL impls (one from def1, one from def2; sparkImpl
filtered out)
+ assertEquals(2, functions.size());
+
+ List<String> sqlBodies =
functions.stream().map(LanguageFunction::sql).sorted().toList();
+ assertEquals("RETURN length(x)", sqlBodies.get(0));
+ assertEquals("RETURN x + 1", sqlBodies.get(1));
+ }
+
+ @Test
+ public void testSignatureTokenFormat() {
+ FunctionParam param1 = createMockParam("a", Types.IntegerType.get());
+ FunctionParam param2 = createMockParam("b", Types.StringType.get());
+
+ FunctionImpl impl = FunctionImpls.ofSql(FunctionImpl.RuntimeType.TRINO,
"RETURN a");
+ FunctionDefinition def = createMockDefinition(new FunctionParam[] {param1,
param2}, impl);
+ Function function = createMockFunctionWithDefinitions("test_func", def);
+
+ CatalogConnectorMetadata catalogMetadata =
mock(CatalogConnectorMetadata.class);
+ when(catalogMetadata.supportsFunctions()).thenReturn(true);
+ when(catalogMetadata.getFunction("s", "test_func")).thenReturn(function);
+
+ GravitinoMetadata metadata = createTestMetadata(catalogMetadata);
+ ConnectorSession session = mock(ConnectorSession.class);
+
+ Collection<LanguageFunction> functions =
+ metadata.getLanguageFunctions(session, new SchemaFunctionName("s",
"test_func"));
+ assertEquals(1, functions.size());
+ assertEquals("test_func(integer,string)",
functions.iterator().next().signatureToken());
Review Comment:
This test asserts a signature token containing the literal type name
`string`. Trino type tokens typically use Trino type names (e.g., `varchar`
rather than `string`), and the connector's `GeneralDataTypeTransformer` maps
Gravitino STRING to Trino VARCHAR. Once signature tokens are generated from
Trino types, update this expectation accordingly.
```suggestion
assertEquals("test_func(integer,varchar)",
functions.iterator().next().signatureToken());
```
--
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]
