mchades commented on code in PR #10855: URL: https://github.com/apache/gravitino/pull/10855#discussion_r3129968497
########## core/src/main/java/org/apache/gravitino/hook/FunctionHookDispatcher.java: ########## @@ -0,0 +1,105 @@ +/* + * 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.hook; + +import org.apache.gravitino.Entity; +import org.apache.gravitino.GravitinoEnv; +import org.apache.gravitino.NameIdentifier; +import org.apache.gravitino.Namespace; +import org.apache.gravitino.authorization.Owner; +import org.apache.gravitino.authorization.OwnerDispatcher; +import org.apache.gravitino.catalog.FunctionDispatcher; +import org.apache.gravitino.exceptions.FunctionAlreadyExistsException; +import org.apache.gravitino.exceptions.NoSuchFunctionException; +import org.apache.gravitino.exceptions.NoSuchSchemaException; +import org.apache.gravitino.function.Function; +import org.apache.gravitino.function.FunctionChange; +import org.apache.gravitino.function.FunctionDefinition; +import org.apache.gravitino.function.FunctionType; +import org.apache.gravitino.utils.NameIdentifierUtil; +import org.apache.gravitino.utils.PrincipalUtils; + +/** + * {@code FunctionHookDispatcher} is a decorator for {@link FunctionDispatcher} that not only + * delegates function operations to the underlying function dispatcher but also executes some hook + * operations before or after the underlying operations. + */ +public class FunctionHookDispatcher implements FunctionDispatcher { + + private final FunctionDispatcher dispatcher; + + public FunctionHookDispatcher(FunctionDispatcher dispatcher) { + this.dispatcher = dispatcher; + } + + @Override + public NameIdentifier[] listFunctions(Namespace namespace) throws NoSuchSchemaException { + return dispatcher.listFunctions(namespace); + } + + @Override + public Function[] listFunctionInfos(Namespace namespace) throws NoSuchSchemaException { + return dispatcher.listFunctionInfos(namespace); + } + + @Override + public Function getFunction(NameIdentifier ident) throws NoSuchFunctionException { + return dispatcher.getFunction(ident); + } + + @Override + public boolean functionExists(NameIdentifier ident) { + return dispatcher.functionExists(ident); + } + + @Override + public Function registerFunction( + NameIdentifier ident, + String comment, + FunctionType functionType, + boolean deterministic, + FunctionDefinition[] definitions) + throws NoSuchSchemaException, FunctionAlreadyExistsException { + Function function = + dispatcher.registerFunction(ident, comment, functionType, deterministic, definitions); + + // Set the creator as owner of the function. + OwnerDispatcher ownerManager = GravitinoEnv.getInstance().ownerDispatcher(); + if (ownerManager != null) { + ownerManager.setOwner( + ident.namespace().level(0), + NameIdentifierUtil.toMetadataObject(ident, Entity.EntityType.FUNCTION), + PrincipalUtils.getCurrentUserName(), + Owner.Type.USER); + } + return function; + } Review Comment: Added core/src/test/java/org/apache/gravitino/hook/TestFunctionHookDispatcher.java. It now verifies OwnerDispatcher#setOwner is called after registerFunction with FUNCTION metadata object, and also verifies registerFunction succeeds when ownerDispatcher is null. ########## core/src/main/java/org/apache/gravitino/storage/relational/service/FunctionMetaService.java: ########## @@ -80,6 +80,24 @@ public FunctionEntity getFunctionByIdentifier(NameIdentifier ident) { return fromFunctionPO(functionPO, ident.namespace()); } + @Monitored( + metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME, + baseMetricName = "getFunctionIdBySchemaIdAndFunctionName") + public Long getFunctionIdBySchemaIdAndFunctionName(Long schemaId, String functionName) { + Long functionId = + SessionUtils.getWithoutCommit( + FunctionMetaMapper.class, + mapper -> mapper.selectFunctionIdBySchemaIdAndFunctionName(schemaId, functionName)); + + if (functionId == null) { + throw new NoSuchEntityException( + NoSuchEntityException.NO_SUCH_ENTITY_MESSAGE, + Entity.EntityType.FUNCTION.name().toLowerCase(Locale.ROOT), + functionName); + } + return functionId; + } Review Comment: Added testGetFunctionIdBySchemaIdAndFunctionName in TestFunctionMetaService. It covers the successful lookup path and NoSuchEntityException for both missing function name and missing schemaId. ########## core/src/main/java/org/apache/gravitino/storage/relational/service/MetadataObjectService.java: ########## @@ -325,6 +328,56 @@ public static Map<Long, String> getModelObjectsFullName(List<Long> modelIds) { return modelIdAndNameMap; } + /** + * Retrieves a map of Function object IDs to their full names. + * + * @param functionIds A list of Function object IDs to fetch names for. + * @return A Map where the key is the Function ID and the value is the Function full name. The map + * may contain null values for the names if its parent object is deleted. Returns an empty map + * if no Function objects are found for the given IDs. {@code @example} value of function full + * name: "catalog1.schema1.function1" + */ + @Monitored( + metricsSource = GRAVITINO_RELATIONAL_STORE_METRIC_NAME, + baseMetricName = "getFunctionObjectsFullName") + public static Map<Long, String> getFunctionObjectsFullName(List<Long> functionIds) { + if (functionIds == null || functionIds.isEmpty()) { + return Maps.newHashMap(); + } + + List<FunctionPO> functionPOs = + SessionUtils.getWithoutCommit( + FunctionMetaMapper.class, mapper -> mapper.listFunctionPOsByFunctionIds(functionIds)); + + if (functionPOs == null || functionPOs.isEmpty()) { + return new HashMap<>(); + } + + List<Long> schemaIds = + functionPOs.stream().map(FunctionPO::schemaId).collect(Collectors.toList()); + + Map<Long, String> schemaIdAndNameMap = getSchemaObjectsFullName(schemaIds); + + HashMap<Long, String> functionIdAndNameMap = new HashMap<>(); + + functionPOs.forEach( + functionPO -> { + // since the schema can be deleted, we need to check the null value, + // and when schema is deleted, we will set fullName of functionPO to null. + String schemaName = schemaIdAndNameMap.getOrDefault(functionPO.schemaId(), null); + if (schemaName == null) { + LOG.warn("The schema of function {} may be deleted", functionPO.functionId()); + functionIdAndNameMap.put(functionPO.functionId(), null); + return; + } + + String fullName = DOT_JOINER.join(schemaName, functionPO.functionName()); + functionIdAndNameMap.put(functionPO.functionId(), fullName); + }); + + return functionIdAndNameMap; + } Review Comment: Added FUNCTION coverage in TestMetadataObjectService for fromGenericEntities: it verifies FUNCTION type + catalog.schema.function fullName, and also adds a missing-parent scenario by soft-deleting schema_meta to confirm invalid entries are skipped consistently. -- 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]
