Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao merged PR #9560: URL: https://github.com/apache/gravitino/pull/9560 -- 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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2694670895
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +160,150 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ private Function doRegisterFunction(
+ NameIdentifier ident,
+ String comment,
+ FunctionType functionType,
+ boolean deterministic,
+ Optional returnType,
+ Optional returnColumns,
+ FunctionDefinition[] definitions)
+ throws NoSuchSchemaException, FunctionAlreadyExistsException {
+Preconditions.checkArgument(
+definitions != null && definitions.length > 0,
+"At least one function definition must be provided");
+validateDefinitionsNoArityOverlap(definitions);
+
+String currentUser = PrincipalUtils.getCurrentUserName();
+Instant now = Instant.now();
+AuditInfo auditInfo =
AuditInfo.builder().withCreator(currentUser).withCreateTime(now).build();
+
+FunctionEntity functionEntity =
+FunctionEntity.builder()
+.withId(idGenerator.nextId())
+.withName(ident.name())
+.withNamespace(ident.namespace())
+.withComment(comment)
+.withFunctionType(functionType)
+.withDeterministic(deterministic)
+.withReturnType(returnType.orElse(null))
+.withReturnColumns(returnColumns.orElse(null))
+.withDefinitions(definitions)
+.withVersion(INIT_VERSION)
+.withAuditInfo(auditInfo)
+.build();
+
+try {
+ store.put(functionEntity, false /* overwrite */);
+ return functionEntity;
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
ident.namespace());
+} catch (EntityAlreadyExistsException e) {
+ throw new FunctionAlreadyExistsException(e, "Function %s already
exists", ident);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to register function " + ident, e);
+}
+ }
+
+ /**
+ * Validates that all definitions in the array do not have overlapping
arities. This is used when
+ * registering a function with multiple definitions.
+ *
+ * Gravitino enforces strict validation to prevent ambiguity. Operations
MUST fail if any
+ * definition's invocation arities overlap with another. For example, if an
existing definition
+ * {@code foo(int, float default 1.0)} supports arities {@code (int)} and
{@code (int, float)},
+ * adding a new definition {@code foo(int, string default 'x')} (which
supports {@code (int)} and
+ * {@code (int, string)}) will be REJECTED because both support the call
{@code foo(1)}. This
+ * ensures every function invocation deterministically maps to a single
definition.
+ *
+ * @param definitions The array of definitions to validate.
+ * @throws IllegalArgumentException If any two definitions have overlapping
arities.
+ */
+ private void validateDefinitionsNoArityOverlap(FunctionDefinition[]
definitions) {
Review Comment:
fixed
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2694672892
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +160,150 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ private Function doRegisterFunction(
+ NameIdentifier ident,
+ String comment,
+ FunctionType functionType,
+ boolean deterministic,
+ Optional returnType,
+ Optional returnColumns,
+ FunctionDefinition[] definitions)
+ throws NoSuchSchemaException, FunctionAlreadyExistsException {
+Preconditions.checkArgument(
+definitions != null && definitions.length > 0,
+"At least one function definition must be provided");
+validateDefinitionsNoArityOverlap(definitions);
+
+String currentUser = PrincipalUtils.getCurrentUserName();
+Instant now = Instant.now();
+AuditInfo auditInfo =
AuditInfo.builder().withCreator(currentUser).withCreateTime(now).build();
+
+FunctionEntity functionEntity =
+FunctionEntity.builder()
+.withId(idGenerator.nextId())
+.withName(ident.name())
+.withNamespace(ident.namespace())
+.withComment(comment)
+.withFunctionType(functionType)
+.withDeterministic(deterministic)
+.withReturnType(returnType.orElse(null))
+.withReturnColumns(returnColumns.orElse(null))
+.withDefinitions(definitions)
+.withVersion(INIT_VERSION)
+.withAuditInfo(auditInfo)
+.build();
+
+try {
+ store.put(functionEntity, false /* overwrite */);
+ return functionEntity;
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
ident.namespace());
+} catch (EntityAlreadyExistsException e) {
+ throw new FunctionAlreadyExistsException(e, "Function %s already
exists", ident);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to register function " + ident, e);
+}
+ }
+
+ /**
+ * Validates that all definitions in the array do not have overlapping
arities. This is used when
+ * registering a function with multiple definitions.
+ *
+ * Gravitino enforces strict validation to prevent ambiguity. Operations
MUST fail if any
+ * definition's invocation arities overlap with another. For example, if an
existing definition
+ * {@code foo(int, float default 1.0)} supports arities {@code (int)} and
{@code (int, float)},
+ * adding a new definition {@code foo(int, string default 'x')} (which
supports {@code (int)} and
+ * {@code (int, string)}) will be REJECTED because both support the call
{@code foo(1)}. This
+ * ensures every function invocation deterministically maps to a single
definition.
+ *
+ * @param definitions The array of definitions to validate.
+ * @throws IllegalArgumentException If any two definitions have overlapping
arities.
+ */
+ private void validateDefinitionsNoArityOverlap(FunctionDefinition[]
definitions) {
+for (int i = 0; i < definitions.length; i++) {
+ Set aritiesI = computeArities(definitions[i]);
+ for (int j = i + 1; j < definitions.length; j++) {
+Set aritiesJ = computeArities(definitions[j]);
+for (String arity : aritiesI) {
+ if (aritiesJ.contains(arity)) {
+throw new IllegalArgumentException(
+String.format(
+"Cannot register function: definitions at index %d and %d
have overlapping "
++ "arity '%s'. This would create ambiguous function
invocations.",
+i, j, arity));
+ }
+}
+ }
+}
+ }
Review Comment:
optimized. plz review again, thx
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2694671583
##
core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java:
##
@@ -0,0 +1,494 @@
+/*
+ * 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.catalog;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityAlreadyExistsException;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.exceptions.FunctionAlreadyExistsException;
+import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NoSuchFunctionException;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionDefinitions;
+import org.apache.gravitino.function.FunctionImpl;
+import org.apache.gravitino.function.FunctionImpls;
+import org.apache.gravitino.function.FunctionParam;
+import org.apache.gravitino.function.FunctionParams;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.meta.FunctionEntity;
+import org.apache.gravitino.rel.expressions.literals.Literals;
+import org.apache.gravitino.rel.types.Types;
+import org.apache.gravitino.storage.IdGenerator;
+import org.apache.gravitino.storage.RandomIdGenerator;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class TestManagedFunctionOperations {
+
+ private static final String METALAKE_NAME = "test_metalake";
+ private static final String CATALOG_NAME = "test_catalog";
+ private static final String SCHEMA_NAME = "schema1";
+
+ private final IdGenerator idGenerator = new RandomIdGenerator();
+ private final Map entityMap = new
HashMap<>();
+
+ private EntityStore store;
+ private ManagedFunctionOperations functionOperations;
+
+ @BeforeEach
+ public void setUp() throws Exception {
+entityMap.clear();
+store = createMockEntityStore();
+functionOperations = new ManagedFunctionOperations(store, idGenerator);
+ }
+
+ @Test
+ public void testRegisterAndListFunctions() {
+NameIdentifier func1Ident = getFunctionIdent("func1");
+FunctionParam[] params1 = new FunctionParam[] {FunctionParams.of("a",
Types.IntegerType.get())};
+FunctionDefinition[] definitions1 = new FunctionDefinition[]
{createSimpleDefinition(params1)};
+
+functionOperations.registerFunction(
+func1Ident,
+"Test function 1",
+FunctionType.SCALAR,
+true,
+Types.StringType.get(),
+definitions1);
+
+NameIdentifier func2Ident = getFunctionIdent("func2");
+FunctionParam[] params2 =
+new FunctionParam[] {
+ FunctionParams.of("x", Types.StringType.get()),
+ FunctionParams.of("y", Types.StringType.get())
+};
+FunctionDefinition[] definitions2 = new FunctionDefinition[]
{createSimpleDefinition(params2)};
+
+functionOperations.registerFunction(
+func2Ident,
+"Test function 2",
+FunctionType.SCALAR,
+false,
+Types.IntegerType.get(),
+definitions2);
+
+// List functions
+NameIdentifier[] functionIdents =
functionOperations.listFunctions(getFunctionNamespace());
+Assertions.assertEquals(2, functionIdents.length);
+Set functionNames =
+
Arrays.stream(functionIdents).map(NameIdentifier::name).collect(Collectors.toSet());
+
+Assertions.assertTrue(functionNames.contains("func1"));
+Assertions.assertTrue(functionNames.contains("func2"));
+ }
+
+ @Test
+ public void testRegisterAndGetFunction() {
+NameIdentifier funcIdent = getFunctionIdent("my_func");
+FunctionParam[] params =
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2694669894
##
api/src/main/java/org/apache/gravitino/function/FunctionCatalog.java:
##
@@ -52,28 +51,17 @@ public interface FunctionCatalog {
/**
* Get a function by {@link NameIdentifier} from the catalog. The identifier
only contains the
* schema and function name. A function may include multiple definitions
(overloads) in the
- * result. This method returns the latest version of the function.
+ * result. This method returns the current version of the function.
*
- * @param ident A function identifier.
- * @return The latest version of the function with the given name.
- * @throws NoSuchFunctionException If the function does not exist.
- */
- Function getFunction(NameIdentifier ident) throws NoSuchFunctionException;
-
- /**
- * Get a function by {@link NameIdentifier} and version from the catalog.
The identifier only
- * contains the schema and function name. A function may include multiple
definitions (overloads)
- * in the result.
+ * Note: Currently, the current version is always the latest version. In
the future, when
+ * rollback functionality is supported, the current version may differ from
the latest version.
*
* @param ident A function identifier.
- * @param version The zero-based function version index (0 for the first
created version), as
- * returned by {@link Function#version()}.
- * @return The function with the given name and version.
+ * @return The current version of the function with the given name.
Review Comment:
removed
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2694526923
##
core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java:
##
@@ -0,0 +1,357 @@
+/*
+ * 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.meta;
+
+import com.google.common.collect.Maps;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+import lombok.ToString;
+import org.apache.gravitino.Auditable;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.Field;
+import org.apache.gravitino.HasIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionColumn;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.rel.types.Type;
+
+/**
+ * A class representing a function entity in the metadata store.
+ *
+ * This entity stores both the function metadata and its version
information together, avoiding
+ * the need for separate FunctionEntity and FunctionVersionEntity. When
retrieving, if version is
+ * set to the special value {@link #LATEST_VERSION}, the store should return
the latest version.
+ */
+@ToString
+public class FunctionEntity implements Entity, Auditable, HasIdentifier,
Function {
+
+ /** Special version value indicating the latest version should be retrieved.
*/
+ public static final int LATEST_VERSION = -1;
+
+ public static final Field ID =
+ Field.required("id", Long.class, "The unique id of the function
entity.");
+ public static final Field NAME =
+ Field.required("name", String.class, "The name of the function entity.");
+ public static final Field COMMENT =
+ Field.optional("comment", String.class, "The comment or description of
the function entity.");
+ public static final Field FUNCTION_TYPE =
+ Field.required("function_type", FunctionType.class, "The type of the
function.");
+ public static final Field DETERMINISTIC =
+ Field.required("deterministic", Boolean.class, "Whether the function is
deterministic.");
+ public static final Field RETURN_TYPE =
+ Field.optional(
+ "return_type", Type.class, "The return type for scalar or aggregate
functions.");
+ public static final Field RETURN_COLUMNS =
+ Field.optional(
+ "return_columns",
+ FunctionColumn[].class,
+ "The output columns for table-valued functions.");
+ public static final Field DEFINITIONS =
+ Field.required("definitions", FunctionDefinition[].class, "The
definitions of the function.");
+ public static final Field VERSION =
+ Field.required("version", Integer.class, "The version of the function
entity.");
+ public static final Field AUDIT_INFO =
+ Field.required("audit_info", AuditInfo.class, "The audit details of the
function entity.");
+
+ private Long id;
+ private String name;
+ private Namespace namespace;
+ private String comment;
+ private FunctionType functionType;
+ private boolean deterministic;
+ private Type returnType;
+ private FunctionColumn[] returnColumns;
+ private FunctionDefinition[] definitions;
+ private Integer version;
+ private AuditInfo auditInfo;
+
+ private FunctionEntity() {}
+
+ @Override
+ public Map fields() {
+Map fields = Maps.newHashMap();
+fields.put(ID, id);
+fields.put(NAME, name);
+fields.put(COMMENT, comment);
+fields.put(FUNCTION_TYPE, functionType);
+fields.put(DETERMINISTIC, deterministic);
+fields.put(RETURN_TYPE, returnType);
+fields.put(RETURN_COLUMNS, returnColumns);
+fields.put(DEFINITIONS, definitions);
+fields.put(VERSION, version);
+fields.put(AUDIT_INFO, auditInfo);
+
+return Collections.unmodifiableMap(fields);
+ }
+
+ @Override
+ public String name() {
+return name;
+ }
+
+ @Override
+ public Long id() {
+return id;
+ }
+
+ @Override
+ public Namespace namespace() {
+return namespace;
+ }
+
+ @Override
+ public String comment() {
+return comment;
+ }
+
+ @Override
+ public FunctionType functionType() {
+return funct
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2694299372
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +160,150 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ private Function doRegisterFunction(
+ NameIdentifier ident,
+ String comment,
+ FunctionType functionType,
+ boolean deterministic,
+ Optional returnType,
+ Optional returnColumns,
+ FunctionDefinition[] definitions)
+ throws NoSuchSchemaException, FunctionAlreadyExistsException {
+Preconditions.checkArgument(
+definitions != null && definitions.length > 0,
+"At least one function definition must be provided");
+validateDefinitionsNoArityOverlap(definitions);
+
+String currentUser = PrincipalUtils.getCurrentUserName();
+Instant now = Instant.now();
+AuditInfo auditInfo =
AuditInfo.builder().withCreator(currentUser).withCreateTime(now).build();
+
+FunctionEntity functionEntity =
+FunctionEntity.builder()
+.withId(idGenerator.nextId())
+.withName(ident.name())
+.withNamespace(ident.namespace())
+.withComment(comment)
+.withFunctionType(functionType)
+.withDeterministic(deterministic)
+.withReturnType(returnType.orElse(null))
+.withReturnColumns(returnColumns.orElse(null))
+.withDefinitions(definitions)
+.withVersion(INIT_VERSION)
+.withAuditInfo(auditInfo)
+.build();
+
+try {
+ store.put(functionEntity, false /* overwrite */);
+ return functionEntity;
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
ident.namespace());
+} catch (EntityAlreadyExistsException e) {
+ throw new FunctionAlreadyExistsException(e, "Function %s already
exists", ident);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to register function " + ident, e);
+}
+ }
+
+ /**
+ * Validates that all definitions in the array do not have overlapping
arities. This is used when
+ * registering a function with multiple definitions.
+ *
+ * Gravitino enforces strict validation to prevent ambiguity. Operations
MUST fail if any
+ * definition's invocation arities overlap with another. For example, if an
existing definition
+ * {@code foo(int, float default 1.0)} supports arities {@code (int)} and
{@code (int, float)},
+ * adding a new definition {@code foo(int, string default 'x')} (which
supports {@code (int)} and
+ * {@code (int, string)}) will be REJECTED because both support the call
{@code foo(1)}. This
+ * ensures every function invocation deterministically maps to a single
definition.
+ *
+ * @param definitions The array of definitions to validate.
+ * @throws IllegalArgumentException If any two definitions have overlapping
arities.
+ */
+ private void validateDefinitionsNoArityOverlap(FunctionDefinition[]
definitions) {
+for (int i = 0; i < definitions.length; i++) {
+ Set aritiesI = computeArities(definitions[i]);
+ for (int j = i + 1; j < definitions.length; j++) {
+Set aritiesJ = computeArities(definitions[j]);
+for (String arity : aritiesI) {
+ if (aritiesJ.contains(arity)) {
+throw new IllegalArgumentException(
+String.format(
+"Cannot register function: definitions at index %d and %d
have overlapping "
++ "arity '%s'. This would create ambiguous function
invocations.",
+i, j, arity));
+ }
+}
+ }
+}
+ }
Review Comment:
My feeling is that the complexity of this method is super high, is it
possible that we can reduce it?
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
Copilot commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2694281834
##
core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java:
##
@@ -0,0 +1,357 @@
+/*
+ * 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.meta;
+
+import com.google.common.collect.Maps;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+import lombok.ToString;
+import org.apache.gravitino.Auditable;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.Field;
+import org.apache.gravitino.HasIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionColumn;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.rel.types.Type;
+
+/**
+ * A class representing a function entity in the metadata store.
+ *
+ * This entity stores both the function metadata and its version
information together, avoiding
+ * the need for separate FunctionEntity and FunctionVersionEntity. When
retrieving, if version is
+ * set to the special value {@link #LATEST_VERSION}, the store should return
the latest version.
+ */
+@ToString
+public class FunctionEntity implements Entity, Auditable, HasIdentifier,
Function {
+
+ /** Special version value indicating the latest version should be retrieved.
*/
+ public static final int LATEST_VERSION = -1;
+
+ public static final Field ID =
+ Field.required("id", Long.class, "The unique id of the function
entity.");
+ public static final Field NAME =
+ Field.required("name", String.class, "The name of the function entity.");
+ public static final Field COMMENT =
+ Field.optional("comment", String.class, "The comment or description of
the function entity.");
+ public static final Field FUNCTION_TYPE =
+ Field.required("function_type", FunctionType.class, "The type of the
function.");
+ public static final Field DETERMINISTIC =
+ Field.required("deterministic", Boolean.class, "Whether the function is
deterministic.");
+ public static final Field RETURN_TYPE =
+ Field.optional(
+ "return_type", Type.class, "The return type for scalar or aggregate
functions.");
+ public static final Field RETURN_COLUMNS =
+ Field.optional(
+ "return_columns",
+ FunctionColumn[].class,
+ "The output columns for table-valued functions.");
+ public static final Field DEFINITIONS =
+ Field.required("definitions", FunctionDefinition[].class, "The
definitions of the function.");
+ public static final Field VERSION =
+ Field.required("version", Integer.class, "The version of the function
entity.");
+ public static final Field AUDIT_INFO =
+ Field.required("audit_info", AuditInfo.class, "The audit details of the
function entity.");
+
+ private Long id;
+ private String name;
+ private Namespace namespace;
+ private String comment;
+ private FunctionType functionType;
+ private boolean deterministic;
+ private Type returnType;
+ private FunctionColumn[] returnColumns;
+ private FunctionDefinition[] definitions;
+ private Integer version;
+ private AuditInfo auditInfo;
+
+ private FunctionEntity() {}
+
+ @Override
+ public Map fields() {
+Map fields = Maps.newHashMap();
+fields.put(ID, id);
+fields.put(NAME, name);
+fields.put(COMMENT, comment);
+fields.put(FUNCTION_TYPE, functionType);
+fields.put(DETERMINISTIC, deterministic);
+fields.put(RETURN_TYPE, returnType);
+fields.put(RETURN_COLUMNS, returnColumns);
+fields.put(DEFINITIONS, definitions);
+fields.put(VERSION, version);
+fields.put(AUDIT_INFO, auditInfo);
+
+return Collections.unmodifiableMap(fields);
+ }
+
+ @Override
+ public String name() {
+return name;
+ }
+
+ @Override
+ public Long id() {
+return id;
+ }
+
+ @Override
+ public Namespace namespace() {
+return namespace;
+ }
+
+ @Override
+ public String comment() {
+return comment;
+ }
+
+ @Override
+ public FunctionType functionType() {
+return funct
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2694275578
##
api/src/main/java/org/apache/gravitino/function/FunctionCatalog.java:
##
@@ -52,28 +51,17 @@ public interface FunctionCatalog {
/**
* Get a function by {@link NameIdentifier} from the catalog. The identifier
only contains the
* schema and function name. A function may include multiple definitions
(overloads) in the
- * result. This method returns the latest version of the function.
+ * result. This method returns the current version of the function.
*
- * @param ident A function identifier.
- * @return The latest version of the function with the given name.
- * @throws NoSuchFunctionException If the function does not exist.
- */
- Function getFunction(NameIdentifier ident) throws NoSuchFunctionException;
-
- /**
- * Get a function by {@link NameIdentifier} and version from the catalog.
The identifier only
- * contains the schema and function name. A function may include multiple
definitions (overloads)
- * in the result.
+ * Note: Currently, the current version is always the latest version. In
the future, when
+ * rollback functionality is supported, the current version may differ from
the latest version.
*
* @param ident A function identifier.
- * @param version The zero-based function version index (0 for the first
created version), as
- * returned by {@link Function#version()}.
- * @return The function with the given name and version.
+ * @return The current version of the function with the given name.
Review Comment:
Also here, it is better not to give user a misinformation if unneeded.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2694270796
##
api/src/main/java/org/apache/gravitino/function/FunctionCatalog.java:
##
@@ -52,28 +51,17 @@ public interface FunctionCatalog {
/**
* Get a function by {@link NameIdentifier} from the catalog. The identifier
only contains the
* schema and function name. A function may include multiple definitions
(overloads) in the
- * result. This method returns the latest version of the function.
+ * result. This method returns the current version of the function.
*
- * @param ident A function identifier.
- * @return The latest version of the function with the given name.
- * @throws NoSuchFunctionException If the function does not exist.
- */
- Function getFunction(NameIdentifier ident) throws NoSuchFunctionException;
-
- /**
- * Get a function by {@link NameIdentifier} and version from the catalog.
The identifier only
- * contains the schema and function name. A function may include multiple
definitions (overloads)
- * in the result.
+ * Note: Currently, the current version is always the latest version. In
the future, when
+ * rollback functionality is supported, the current version may differ from
the latest version.
Review Comment:
We don't have to emphasize the version thing here, since we don't actually
support any version-related operations. Can you update the javadoc here and the
function definition?
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2681029578
##
core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java:
##
@@ -0,0 +1,357 @@
+/*
+ * 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.meta;
+
+import com.google.common.collect.Maps;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+import lombok.ToString;
+import org.apache.gravitino.Auditable;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.Field;
+import org.apache.gravitino.HasIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionColumn;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.rel.types.Type;
+
+/**
+ * A class representing a function entity in the metadata store.
+ *
+ * This entity stores both the function metadata and its version
information together, avoiding
+ * the need for separate FunctionEntity and FunctionVersionEntity. When
retrieving, if version is
+ * set to the special value {@link #LATEST_VERSION}, the store should return
the latest version.
+ */
+@ToString
+public class FunctionEntity implements Entity, Auditable, HasIdentifier,
Function {
+
+ /** Special version value indicating the latest version should be retrieved.
*/
+ public static final int LATEST_VERSION = -1;
+
+ public static final Field ID =
+ Field.required("id", Long.class, "The unique id of the function
entity.");
+ public static final Field NAME =
+ Field.required("name", String.class, "The name of the function entity.");
+ public static final Field COMMENT =
+ Field.optional("comment", String.class, "The comment or description of
the function entity.");
+ public static final Field FUNCTION_TYPE =
+ Field.required("function_type", FunctionType.class, "The type of the
function.");
+ public static final Field DETERMINISTIC =
+ Field.required("deterministic", Boolean.class, "Whether the function is
deterministic.");
+ public static final Field RETURN_TYPE =
+ Field.optional(
+ "return_type", Type.class, "The return type for scalar or aggregate
functions.");
+ public static final Field RETURN_COLUMNS =
+ Field.optional(
+ "return_columns",
+ FunctionColumn[].class,
+ "The output columns for table-valued functions.");
+ public static final Field DEFINITIONS =
+ Field.required("definitions", FunctionDefinition[].class, "The
definitions of the function.");
+ public static final Field VERSION =
+ Field.required("version", Integer.class, "The version of the function
entity.");
Review Comment:
The changes have already been implemented as discussed.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2680993767
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +170,173 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ /**
+ * Converts a function identifier to a versioned identifier. The versioned
identifier uses the
+ * version number as the name to allow the store to retrieve specific
versions.
+ *
+ * @param ident The function identifier.
+ * @param version The version number, or {@link
FunctionEntity#LATEST_VERSION} for the latest.
+ * @return The versioned identifier.
+ */
+ private NameIdentifier toVersionedIdent(NameIdentifier ident, int version) {
+return NameIdentifier.of(
+ident.namespace().level(0),
+ident.namespace().level(1),
+ident.namespace().level(2),
+ident.name(),
+String.valueOf(version));
+ }
+
+ private Function doRegisterFunction(
+ NameIdentifier ident,
+ String comment,
+ FunctionType functionType,
+ boolean deterministic,
+ Type returnType,
+ FunctionColumn[] returnColumns,
+ FunctionDefinition[] definitions)
+ throws NoSuchSchemaException, FunctionAlreadyExistsException {
+Preconditions.checkArgument(
+definitions != null && definitions.length > 0,
+"At least one function definition must be provided");
+validateDefinitionsNoArityOverlap(definitions);
+
+String currentUser = PrincipalUtils.getCurrentUserName();
+Instant now = Instant.now();
+AuditInfo auditInfo =
AuditInfo.builder().withCreator(currentUser).withCreateTime(now).build();
+
+FunctionEntity functionEntity =
+FunctionEntity.builder()
+.withId(idGenerator.nextId())
+.withName(ident.name())
+.withNamespace(ident.namespace())
+.withComment(comment)
+.withFunctionType(functionType)
+.withDeterministic(deterministic)
+.withReturnType(returnType)
+.withReturnColumns(returnColumns)
+.withDefinitions(definitions)
+.withVersion(INIT_VERSION)
+.withAuditInfo(auditInfo)
+.build();
+
+try {
+ store.put(functionEntity, false /* overwrite */);
+ return functionEntity;
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
ident.namespace());
+} catch (EntityAlreadyExistsException e) {
+ throw new FunctionAlreadyExistsException(e, "Function %s already
exists", ident);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to register function " + ident, e);
+}
+ }
+
+ /**
+ * Validates that all definitions in the array do not have overlapping
arities. This is used when
+ * registering a function with multiple definitions.
+ *
+ * Gravitino enforces strict validation to prevent ambiguity. Operations
MUST fail if any
+ * definition's invocation arities overlap with another. For example, if an
existing definition
+ * {@code foo(int, float default 1.0)} supports arities {@code (int)} and
{@code (int, float)},
+ * adding a new definition {@code foo(int, string default 'x')} (which
supports {@code (int)} and
+ * {@code (int, string)}) will be REJECTED because both support the call
{@code foo(1)}. This
+ * ensures every function invocation deterministically maps to a single
definition.
+ *
+ * @param definitions The array of definitions to validate.
+ * @throws IllegalArgumentException If any two definitions have overlapping
arities.
+ */
+ private void validateDefinitionsNoArityOverlap(FunctionDefinition[]
definitions) {
Review Comment:
Yes. The tests `testRegisterFunctionWithOverlappingDefinitions`,
`testInvalidParameterOrder`, `testNonOverlappingDefinitions`, and
`testOverlappingAritiesWithDifferentTypes` can cover this method.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2680822594
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +170,173 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ /**
+ * Converts a function identifier to a versioned identifier. The versioned
identifier uses the
+ * version number as the name to allow the store to retrieve specific
versions.
+ *
+ * @param ident The function identifier.
+ * @param version The version number, or {@link
FunctionEntity#LATEST_VERSION} for the latest.
+ * @return The versioned identifier.
+ */
+ private NameIdentifier toVersionedIdent(NameIdentifier ident, int version) {
+return NameIdentifier.of(
+ident.namespace().level(0),
+ident.namespace().level(1),
+ident.namespace().level(2),
+ident.name(),
+String.valueOf(version));
+ }
+
+ private Function doRegisterFunction(
+ NameIdentifier ident,
+ String comment,
+ FunctionType functionType,
+ boolean deterministic,
+ Type returnType,
+ FunctionColumn[] returnColumns,
+ FunctionDefinition[] definitions)
+ throws NoSuchSchemaException, FunctionAlreadyExistsException {
+Preconditions.checkArgument(
+definitions != null && definitions.length > 0,
+"At least one function definition must be provided");
+validateDefinitionsNoArityOverlap(definitions);
+
+String currentUser = PrincipalUtils.getCurrentUserName();
+Instant now = Instant.now();
+AuditInfo auditInfo =
AuditInfo.builder().withCreator(currentUser).withCreateTime(now).build();
+
+FunctionEntity functionEntity =
+FunctionEntity.builder()
+.withId(idGenerator.nextId())
+.withName(ident.name())
+.withNamespace(ident.namespace())
+.withComment(comment)
+.withFunctionType(functionType)
+.withDeterministic(deterministic)
+.withReturnType(returnType)
+.withReturnColumns(returnColumns)
+.withDefinitions(definitions)
+.withVersion(INIT_VERSION)
+.withAuditInfo(auditInfo)
+.build();
+
+try {
+ store.put(functionEntity, false /* overwrite */);
+ return functionEntity;
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
ident.namespace());
+} catch (EntityAlreadyExistsException e) {
+ throw new FunctionAlreadyExistsException(e, "Function %s already
exists", ident);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to register function " + ident, e);
+}
+ }
+
+ /**
+ * Validates that all definitions in the array do not have overlapping
arities. This is used when
+ * registering a function with multiple definitions.
+ *
+ * Gravitino enforces strict validation to prevent ambiguity. Operations
MUST fail if any
+ * definition's invocation arities overlap with another. For example, if an
existing definition
+ * {@code foo(int, float default 1.0)} supports arities {@code (int)} and
{@code (int, float)},
+ * adding a new definition {@code foo(int, string default 'x')} (which
supports {@code (int)} and
+ * {@code (int, string)}) will be REJECTED because both support the call
{@code foo(1)}. This
+ * ensures every function invocation deterministically maps to a single
definition.
+ *
+ * @param definitions The array of definitions to validate.
+ * @throws IllegalArgumentException If any two definitions have overlapping
arities.
+ */
+ private void validateDefinitionsNoArityOverlap(FunctionDefinition[]
definitions) {
Review Comment:
Did you fix this comment?
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2680786814
##
core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java:
##
@@ -0,0 +1,357 @@
+/*
+ * 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.meta;
+
+import com.google.common.collect.Maps;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+import lombok.ToString;
+import org.apache.gravitino.Auditable;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.Field;
+import org.apache.gravitino.HasIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionColumn;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.rel.types.Type;
+
+/**
+ * A class representing a function entity in the metadata store.
+ *
+ * This entity stores both the function metadata and its version
information together, avoiding
+ * the need for separate FunctionEntity and FunctionVersionEntity. When
retrieving, if version is
+ * set to the special value {@link #LATEST_VERSION}, the store should return
the latest version.
+ */
+@ToString
+public class FunctionEntity implements Entity, Auditable, HasIdentifier,
Function {
+
+ /** Special version value indicating the latest version should be retrieved.
*/
+ public static final int LATEST_VERSION = -1;
+
+ public static final Field ID =
+ Field.required("id", Long.class, "The unique id of the function
entity.");
+ public static final Field NAME =
+ Field.required("name", String.class, "The name of the function entity.");
+ public static final Field COMMENT =
+ Field.optional("comment", String.class, "The comment or description of
the function entity.");
+ public static final Field FUNCTION_TYPE =
+ Field.required("function_type", FunctionType.class, "The type of the
function.");
+ public static final Field DETERMINISTIC =
+ Field.required("deterministic", Boolean.class, "Whether the function is
deterministic.");
+ public static final Field RETURN_TYPE =
+ Field.optional(
+ "return_type", Type.class, "The return type for scalar or aggregate
functions.");
+ public static final Field RETURN_COLUMNS =
+ Field.optional(
+ "return_columns",
+ FunctionColumn[].class,
+ "The output columns for table-valued functions.");
+ public static final Field DEFINITIONS =
+ Field.required("definitions", FunctionDefinition[].class, "The
definitions of the function.");
+ public static final Field VERSION =
+ Field.required("version", Integer.class, "The version of the function
entity.");
Review Comment:
As we discussed offline. We can support multi-versions in the storage level.
But from the user-level, they don't have to know more about the versions, they
can use the latest version by default, and they can rollback to a specific
version if they wanted.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on PR #9560: URL: https://github.com/apache/gravitino/pull/9560#issuecomment-3727847273 @jerryshao all comments resolved, plz help to review again when you have time. ty! -- 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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2675322345
##
core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java:
##
@@ -0,0 +1,357 @@
+/*
+ * 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.meta;
+
+import com.google.common.collect.Maps;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+import lombok.ToString;
+import org.apache.gravitino.Auditable;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.Field;
+import org.apache.gravitino.HasIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionColumn;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.rel.types.Type;
+
+/**
+ * A class representing a function entity in the metadata store.
+ *
+ * This entity stores both the function metadata and its version
information together, avoiding
+ * the need for separate FunctionEntity and FunctionVersionEntity. When
retrieving, if version is
+ * set to the special value {@link #LATEST_VERSION}, the store should return
the latest version.
+ */
+@ToString
+public class FunctionEntity implements Entity, Auditable, HasIdentifier,
Function {
+
+ /** Special version value indicating the latest version should be retrieved.
*/
+ public static final int LATEST_VERSION = -1;
+
+ public static final Field ID =
+ Field.required("id", Long.class, "The unique id of the function
entity.");
+ public static final Field NAME =
+ Field.required("name", String.class, "The name of the function entity.");
+ public static final Field COMMENT =
+ Field.optional("comment", String.class, "The comment or description of
the function entity.");
+ public static final Field FUNCTION_TYPE =
+ Field.required("function_type", FunctionType.class, "The type of the
function.");
+ public static final Field DETERMINISTIC =
+ Field.required("deterministic", Boolean.class, "Whether the function is
deterministic.");
+ public static final Field RETURN_TYPE =
+ Field.optional(
+ "return_type", Type.class, "The return type for scalar or aggregate
functions.");
+ public static final Field RETURN_COLUMNS =
+ Field.optional(
+ "return_columns",
+ FunctionColumn[].class,
+ "The output columns for table-valued functions.");
+ public static final Field DEFINITIONS =
+ Field.required("definitions", FunctionDefinition[].class, "The
definitions of the function.");
+ public static final Field VERSION =
+ Field.required("version", Integer.class, "The version of the function
entity.");
Review Comment:
During the initial research phase, Function support for multiple versions
was identified as one of the potential requirements, so I added a version field
in the Function.
However, as you mentioned, the current API implementation hasn't fully
considered the operations for multiple versions.
Therefore, I plan to:
- first implement it in Phase One by displaying the current version of the
function at the API level within the `Function`, while supporting multi-version
storage at the storage level.
- In Phase Two, I will add more comprehensive Function version operations,
such as `listing versions` and `rolling back`.
WDYT?
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2675184228
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +170,173 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ /**
+ * Converts a function identifier to a versioned identifier. The versioned
identifier uses the
+ * version number as the name to allow the store to retrieve specific
versions.
+ *
+ * @param ident The function identifier.
+ * @param version The version number, or {@link
FunctionEntity#LATEST_VERSION} for the latest.
+ * @return The versioned identifier.
+ */
+ private NameIdentifier toVersionedIdent(NameIdentifier ident, int version) {
+return NameIdentifier.of(
+ident.namespace().level(0),
+ident.namespace().level(1),
+ident.namespace().level(2),
+ident.name(),
+String.valueOf(version));
+ }
+
+ private Function doRegisterFunction(
+ NameIdentifier ident,
+ String comment,
+ FunctionType functionType,
+ boolean deterministic,
+ Type returnType,
+ FunctionColumn[] returnColumns,
+ FunctionDefinition[] definitions)
+ throws NoSuchSchemaException, FunctionAlreadyExistsException {
+Preconditions.checkArgument(
+definitions != null && definitions.length > 0,
+"At least one function definition must be provided");
+validateDefinitionsNoArityOverlap(definitions);
+
+String currentUser = PrincipalUtils.getCurrentUserName();
+Instant now = Instant.now();
+AuditInfo auditInfo =
AuditInfo.builder().withCreator(currentUser).withCreateTime(now).build();
+
+FunctionEntity functionEntity =
+FunctionEntity.builder()
+.withId(idGenerator.nextId())
+.withName(ident.name())
+.withNamespace(ident.namespace())
+.withComment(comment)
+.withFunctionType(functionType)
+.withDeterministic(deterministic)
+.withReturnType(returnType)
+.withReturnColumns(returnColumns)
+.withDefinitions(definitions)
+.withVersion(INIT_VERSION)
+.withAuditInfo(auditInfo)
+.build();
+
+try {
+ store.put(functionEntity, false /* overwrite */);
+ return functionEntity;
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
ident.namespace());
+} catch (EntityAlreadyExistsException e) {
+ throw new FunctionAlreadyExistsException(e, "Function %s already
exists", ident);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to register function " + ident, e);
+}
+ }
+
+ /**
+ * Validates that all definitions in the array do not have overlapping
arities. This is used when
+ * registering a function with multiple definitions.
+ *
+ * Gravitino enforces strict validation to prevent ambiguity. Operations
MUST fail if any
+ * definition's invocation arities overlap with another. For example, if an
existing definition
+ * {@code foo(int, float default 1.0)} supports arities {@code (int)} and
{@code (int, float)},
+ * adding a new definition {@code foo(int, string default 'x')} (which
supports {@code (int)} and
+ * {@code (int, string)}) will be REJECTED because both support the call
{@code foo(1)}. This
+ * ensures every function invocation deterministically maps to a single
definition.
+ *
+ * @param definitions The array of definitions to validate.
+ * @throws IllegalArgumentException If any two definitions have overlapping
arities.
+ */
+ private void validateDefinitionsNoArityOverlap(FunctionDefinition[]
definitions) {
+for (int i = 0; i < definitions.length; i++) {
+ Set aritiesI = computeArities(definitions[i]);
+ for (int j = i + 1; j < definitions.length; j++) {
+Set aritiesJ = computeArities(definitions[j]);
+for (String arity : aritiesI) {
+ if (aritiesJ.contains(arity)) {
+throw new IllegalArgumentException(
+String.format(
+"Cannot register function: definitions at index %d and %d
have overlapping "
++ "arity '%s'. This would create ambiguous function
invocations.",
+i, j, arity));
+ }
+}
+ }
+}
+ }
+
+ /**
+ * Computes all possible invocation arities for a functio
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2675129850
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +170,173 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
Review Comment:
Yes. The version of the function is auto-incrementing with each alteration
operation. The plan is to support rolling back the function to a specified
version in the future, so this deletion will remove all versions of the
function.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672260993
##
core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java:
##
@@ -0,0 +1,390 @@
+/*
+ * 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.catalog;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityAlreadyExistsException;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.exceptions.FunctionAlreadyExistsException;
+import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NoSuchFunctionException;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionDefinitions;
+import org.apache.gravitino.function.FunctionImpl;
+import org.apache.gravitino.function.FunctionImpls;
+import org.apache.gravitino.function.FunctionParam;
+import org.apache.gravitino.function.FunctionParams;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.meta.FunctionEntity;
+import org.apache.gravitino.rel.expressions.literals.Literals;
+import org.apache.gravitino.rel.types.Types;
+import org.apache.gravitino.storage.IdGenerator;
+import org.apache.gravitino.storage.RandomIdGenerator;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class TestManagedFunctionOperations {
+
+ private static final String METALAKE_NAME = "test_metalake";
+ private static final String CATALOG_NAME = "test_catalog";
+ private static final String SCHEMA_NAME = "schema1";
+
+ private final IdGenerator idGenerator = new RandomIdGenerator();
+ private final Map entityMap = new
HashMap<>();
+
+ private EntityStore store;
+ private ManagedFunctionOperations functionOperations;
+
+ @BeforeEach
+ public void setUp() throws Exception {
+entityMap.clear();
+store = createMockEntityStore();
+functionOperations = new ManagedFunctionOperations(store, idGenerator);
+ }
+
+ @Test
+ public void testRegisterAndListFunctions() {
+NameIdentifier func1Ident = getFunctionIdent("func1");
+FunctionParam[] params1 = new FunctionParam[] {FunctionParams.of("a",
Types.IntegerType.get())};
+FunctionDefinition[] definitions1 = new FunctionDefinition[]
{createSimpleDefinition(params1)};
+
+functionOperations.registerFunction(
+func1Ident,
+"Test function 1",
+FunctionType.SCALAR,
+true,
+Types.StringType.get(),
+definitions1);
+
+NameIdentifier func2Ident = getFunctionIdent("func2");
+FunctionParam[] params2 =
+new FunctionParam[] {
+ FunctionParams.of("x", Types.StringType.get()),
+ FunctionParams.of("y", Types.StringType.get())
+};
+FunctionDefinition[] definitions2 = new FunctionDefinition[]
{createSimpleDefinition(params2)};
+
+functionOperations.registerFunction(
+func2Ident,
+"Test function 2",
+FunctionType.SCALAR,
+false,
+Types.IntegerType.get(),
+definitions2);
+
+// List functions
+NameIdentifier[] functionIdents =
functionOperations.listFunctions(getFunctionNamespace());
+Assertions.assertEquals(2, functionIdents.length);
+Set functionNames =
+
Arrays.stream(functionIdents).map(NameIdentifier::name).collect(Collectors.toSet());
+
+Assertions.assertTrue(functionNames.contains("func1"));
+Assertions.assertTrue(functionNames.contains("func2"));
+ }
+
+ @Test
+ public void testRegisterAndGetFunction() {
+NameIdentifier funcIdent = getFunctionIdent("my_func");
+FunctionParam[] params
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672256311
##
core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java:
##
@@ -0,0 +1,390 @@
+/*
+ * 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.catalog;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.doAnswer;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.stream.Collectors;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.EntityAlreadyExistsException;
+import org.apache.gravitino.EntityStore;
+import org.apache.gravitino.NameIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.exceptions.FunctionAlreadyExistsException;
+import org.apache.gravitino.exceptions.NoSuchEntityException;
+import org.apache.gravitino.exceptions.NoSuchFunctionException;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionDefinitions;
+import org.apache.gravitino.function.FunctionImpl;
+import org.apache.gravitino.function.FunctionImpls;
+import org.apache.gravitino.function.FunctionParam;
+import org.apache.gravitino.function.FunctionParams;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.meta.FunctionEntity;
+import org.apache.gravitino.rel.expressions.literals.Literals;
+import org.apache.gravitino.rel.types.Types;
+import org.apache.gravitino.storage.IdGenerator;
+import org.apache.gravitino.storage.RandomIdGenerator;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+
+public class TestManagedFunctionOperations {
+
+ private static final String METALAKE_NAME = "test_metalake";
+ private static final String CATALOG_NAME = "test_catalog";
+ private static final String SCHEMA_NAME = "schema1";
+
+ private final IdGenerator idGenerator = new RandomIdGenerator();
+ private final Map entityMap = new
HashMap<>();
+
+ private EntityStore store;
+ private ManagedFunctionOperations functionOperations;
+
+ @BeforeEach
+ public void setUp() throws Exception {
+entityMap.clear();
+store = createMockEntityStore();
+functionOperations = new ManagedFunctionOperations(store, idGenerator);
+ }
+
+ @Test
+ public void testRegisterAndListFunctions() {
+NameIdentifier func1Ident = getFunctionIdent("func1");
+FunctionParam[] params1 = new FunctionParam[] {FunctionParams.of("a",
Types.IntegerType.get())};
+FunctionDefinition[] definitions1 = new FunctionDefinition[]
{createSimpleDefinition(params1)};
+
+functionOperations.registerFunction(
+func1Ident,
+"Test function 1",
+FunctionType.SCALAR,
+true,
+Types.StringType.get(),
+definitions1);
+
+NameIdentifier func2Ident = getFunctionIdent("func2");
+FunctionParam[] params2 =
+new FunctionParam[] {
+ FunctionParams.of("x", Types.StringType.get()),
+ FunctionParams.of("y", Types.StringType.get())
+};
+FunctionDefinition[] definitions2 = new FunctionDefinition[]
{createSimpleDefinition(params2)};
+
+functionOperations.registerFunction(
+func2Ident,
+"Test function 2",
+FunctionType.SCALAR,
+false,
+Types.IntegerType.get(),
+definitions2);
+
+// List functions
+NameIdentifier[] functionIdents =
functionOperations.listFunctions(getFunctionNamespace());
+Assertions.assertEquals(2, functionIdents.length);
+Set functionNames =
+
Arrays.stream(functionIdents).map(NameIdentifier::name).collect(Collectors.toSet());
+
+Assertions.assertTrue(functionNames.contains("func1"));
+Assertions.assertTrue(functionNames.contains("func2"));
+ }
+
+ @Test
+ public void testRegisterAndGetFunction() {
+NameIdentifier funcIdent = getFunctionIdent("my_func");
+FunctionParam[] params
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672234093
##
core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java:
##
@@ -0,0 +1,357 @@
+/*
+ * 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.meta;
+
+import com.google.common.collect.Maps;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+import lombok.ToString;
+import org.apache.gravitino.Auditable;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.Field;
+import org.apache.gravitino.HasIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionColumn;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.rel.types.Type;
+
+/**
+ * A class representing a function entity in the metadata store.
+ *
+ * This entity stores both the function metadata and its version
information together, avoiding
+ * the need for separate FunctionEntity and FunctionVersionEntity. When
retrieving, if version is
+ * set to the special value {@link #LATEST_VERSION}, the store should return
the latest version.
+ */
+@ToString
+public class FunctionEntity implements Entity, Auditable, HasIdentifier,
Function {
+
+ /** Special version value indicating the latest version should be retrieved.
*/
+ public static final int LATEST_VERSION = -1;
+
+ public static final Field ID =
+ Field.required("id", Long.class, "The unique id of the function
entity.");
+ public static final Field NAME =
+ Field.required("name", String.class, "The name of the function entity.");
+ public static final Field COMMENT =
+ Field.optional("comment", String.class, "The comment or description of
the function entity.");
+ public static final Field FUNCTION_TYPE =
+ Field.required("function_type", FunctionType.class, "The type of the
function.");
+ public static final Field DETERMINISTIC =
+ Field.required("deterministic", Boolean.class, "Whether the function is
deterministic.");
+ public static final Field RETURN_TYPE =
+ Field.optional(
+ "return_type", Type.class, "The return type for scalar or aggregate
functions.");
+ public static final Field RETURN_COLUMNS =
+ Field.optional(
+ "return_columns",
+ FunctionColumn[].class,
+ "The output columns for table-valued functions.");
+ public static final Field DEFINITIONS =
+ Field.required("definitions", FunctionDefinition[].class, "The
definitions of the function.");
+ public static final Field VERSION =
+ Field.required("version", Integer.class, "The version of the function
entity.");
Review Comment:
I was think the necessity of version support for UDF. If we want to support
versions, we should let users know the existing versions, and can fetch the
versions, otherwise, how do they know which version to use?
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672162632
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +170,173 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ /**
+ * Converts a function identifier to a versioned identifier. The versioned
identifier uses the
+ * version number as the name to allow the store to retrieve specific
versions.
+ *
+ * @param ident The function identifier.
+ * @param version The version number, or {@link
FunctionEntity#LATEST_VERSION} for the latest.
+ * @return The versioned identifier.
+ */
+ private NameIdentifier toVersionedIdent(NameIdentifier ident, int version) {
+return NameIdentifier.of(
+ident.namespace().level(0),
+ident.namespace().level(1),
+ident.namespace().level(2),
+ident.name(),
+String.valueOf(version));
+ }
+
+ private Function doRegisterFunction(
+ NameIdentifier ident,
+ String comment,
+ FunctionType functionType,
+ boolean deterministic,
+ Type returnType,
+ FunctionColumn[] returnColumns,
+ FunctionDefinition[] definitions)
+ throws NoSuchSchemaException, FunctionAlreadyExistsException {
+Preconditions.checkArgument(
+definitions != null && definitions.length > 0,
+"At least one function definition must be provided");
+validateDefinitionsNoArityOverlap(definitions);
+
+String currentUser = PrincipalUtils.getCurrentUserName();
+Instant now = Instant.now();
+AuditInfo auditInfo =
AuditInfo.builder().withCreator(currentUser).withCreateTime(now).build();
+
+FunctionEntity functionEntity =
+FunctionEntity.builder()
+.withId(idGenerator.nextId())
+.withName(ident.name())
+.withNamespace(ident.namespace())
+.withComment(comment)
+.withFunctionType(functionType)
+.withDeterministic(deterministic)
+.withReturnType(returnType)
+.withReturnColumns(returnColumns)
+.withDefinitions(definitions)
+.withVersion(INIT_VERSION)
+.withAuditInfo(auditInfo)
+.build();
+
+try {
+ store.put(functionEntity, false /* overwrite */);
+ return functionEntity;
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
ident.namespace());
+} catch (EntityAlreadyExistsException e) {
+ throw new FunctionAlreadyExistsException(e, "Function %s already
exists", ident);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to register function " + ident, e);
+}
+ }
+
+ /**
+ * Validates that all definitions in the array do not have overlapping
arities. This is used when
+ * registering a function with multiple definitions.
+ *
+ * Gravitino enforces strict validation to prevent ambiguity. Operations
MUST fail if any
+ * definition's invocation arities overlap with another. For example, if an
existing definition
+ * {@code foo(int, float default 1.0)} supports arities {@code (int)} and
{@code (int, float)},
+ * adding a new definition {@code foo(int, string default 'x')} (which
supports {@code (int)} and
+ * {@code (int, string)}) will be REJECTED because both support the call
{@code foo(1)}. This
+ * ensures every function invocation deterministically maps to a single
definition.
+ *
+ * @param definitions The array of definitions to validate.
+ * @throws IllegalArgumentException If any two definitions have overlapping
arities.
+ */
+ private void validateDefinitionsNoArityOverlap(FunctionDefinition[]
definitions) {
+for (int i = 0; i < definitions.length; i++) {
+ Set aritiesI = computeArities(definitions[i]);
+ for (int j = i + 1; j < definitions.length; j++) {
+Set aritiesJ = computeArities(definitions[j]);
+for (String arity : aritiesI) {
+ if (aritiesJ.contains(arity)) {
+throw new IllegalArgumentException(
+String.format(
+"Cannot register function: definitions at index %d and %d
have overlapping "
++ "arity '%s'. This would create ambiguous function
invocations.",
+i, j, arity));
+ }
+}
+ }
+}
+ }
+
+ /**
+ * Computes all possible invocation arities for a funct
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672132340
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +170,173 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ /**
+ * Converts a function identifier to a versioned identifier. The versioned
identifier uses the
+ * version number as the name to allow the store to retrieve specific
versions.
+ *
+ * @param ident The function identifier.
+ * @param version The version number, or {@link
FunctionEntity#LATEST_VERSION} for the latest.
+ * @return The versioned identifier.
+ */
+ private NameIdentifier toVersionedIdent(NameIdentifier ident, int version) {
+return NameIdentifier.of(
+ident.namespace().level(0),
+ident.namespace().level(1),
+ident.namespace().level(2),
+ident.name(),
+String.valueOf(version));
+ }
+
+ private Function doRegisterFunction(
+ NameIdentifier ident,
+ String comment,
+ FunctionType functionType,
+ boolean deterministic,
+ Type returnType,
+ FunctionColumn[] returnColumns,
+ FunctionDefinition[] definitions)
+ throws NoSuchSchemaException, FunctionAlreadyExistsException {
+Preconditions.checkArgument(
+definitions != null && definitions.length > 0,
+"At least one function definition must be provided");
+validateDefinitionsNoArityOverlap(definitions);
+
+String currentUser = PrincipalUtils.getCurrentUserName();
+Instant now = Instant.now();
+AuditInfo auditInfo =
AuditInfo.builder().withCreator(currentUser).withCreateTime(now).build();
+
+FunctionEntity functionEntity =
+FunctionEntity.builder()
+.withId(idGenerator.nextId())
+.withName(ident.name())
+.withNamespace(ident.namespace())
+.withComment(comment)
+.withFunctionType(functionType)
+.withDeterministic(deterministic)
+.withReturnType(returnType)
+.withReturnColumns(returnColumns)
+.withDefinitions(definitions)
+.withVersion(INIT_VERSION)
+.withAuditInfo(auditInfo)
+.build();
+
+try {
+ store.put(functionEntity, false /* overwrite */);
+ return functionEntity;
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
ident.namespace());
+} catch (EntityAlreadyExistsException e) {
+ throw new FunctionAlreadyExistsException(e, "Function %s already
exists", ident);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to register function " + ident, e);
+}
+ }
+
+ /**
+ * Validates that all definitions in the array do not have overlapping
arities. This is used when
+ * registering a function with multiple definitions.
+ *
+ * Gravitino enforces strict validation to prevent ambiguity. Operations
MUST fail if any
+ * definition's invocation arities overlap with another. For example, if an
existing definition
+ * {@code foo(int, float default 1.0)} supports arities {@code (int)} and
{@code (int, float)},
+ * adding a new definition {@code foo(int, string default 'x')} (which
supports {@code (int)} and
+ * {@code (int, string)}) will be REJECTED because both support the call
{@code foo(1)}. This
+ * ensures every function invocation deterministically maps to a single
definition.
+ *
+ * @param definitions The array of definitions to validate.
+ * @throws IllegalArgumentException If any two definitions have overlapping
arities.
+ */
+ private void validateDefinitionsNoArityOverlap(FunctionDefinition[]
definitions) {
Review Comment:
I would suggest you to add more tests for this method.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672115748
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -117,9 +157,8 @@ public Function registerFunction(
FunctionColumn[] returnColumns,
FunctionDefinition[] definitions)
throws NoSuchSchemaException, FunctionAlreadyExistsException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"registerFunction for table-valued functions: FunctionEntity not yet
implemented");
+return doRegisterFunction(
+ident, comment, FunctionType.TABLE, deterministic, null,
returnColumns, definitions);
Review Comment:
So, for each register, it will store into a new version? Please clarify the
behavior.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672115748
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -117,9 +157,8 @@ public Function registerFunction(
FunctionColumn[] returnColumns,
FunctionDefinition[] definitions)
throws NoSuchSchemaException, FunctionAlreadyExistsException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"registerFunction for table-valued functions: FunctionEntity not yet
implemented");
+return doRegisterFunction(
+ident, comment, FunctionType.TABLE, deterministic, null,
returnColumns, definitions);
Review Comment:
So, for each register, it will store into a new version? Please clarify the
behavior.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672110296
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -131,7 +170,173 @@ public Function alterFunction(NameIdentifier ident,
FunctionChange... changes)
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
Review Comment:
Will this drop all the versions?
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672091185
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -117,9 +157,8 @@ public Function registerFunction(
FunctionColumn[] returnColumns,
FunctionDefinition[] definitions)
throws NoSuchSchemaException, FunctionAlreadyExistsException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"registerFunction for table-valued functions: FunctionEntity not yet
implemented");
+return doRegisterFunction(
+ident, comment, FunctionType.TABLE, deterministic, null,
returnColumns, definitions);
Review Comment:
I'm inclined to use `Optional` instead of null, which has no type
information.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672081159
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -71,29 +84,56 @@ public ManagedFunctionOperations(EntityStore store,
IdGenerator idGenerator) {
@Override
public NameIdentifier[] listFunctions(Namespace namespace) throws
NoSuchSchemaException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("listFunctions: FunctionEntity not
yet implemented");
+return Arrays.stream(listFunctionInfos(namespace))
+.map(f -> NameIdentifier.of(namespace, f.name()))
+.toArray(NameIdentifier[]::new);
}
@Override
public Function[] listFunctionInfos(Namespace namespace) throws
NoSuchSchemaException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"listFunctionInfos: FunctionEntity not yet implemented");
+try {
+ List functions =
+ store.list(namespace, FunctionEntity.class,
Entity.EntityType.FUNCTION);
+ return functions.toArray(FunctionEntity[]::new);
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
namespace);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to list functions in namespace " +
namespace, e);
+}
}
@Override
public Function getFunction(NameIdentifier ident) throws
NoSuchFunctionException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("getFunction: FunctionEntity not
yet implemented");
+return getFunction(ident, FunctionEntity.LATEST_VERSION);
}
@Override
public Function getFunction(NameIdentifier ident, int version)
throws NoSuchFunctionException, NoSuchFunctionVersionException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"getFunction with version: FunctionEntity not yet implemented");
+NameIdentifier versionedIdent = toVersionedIdent(ident, version);
+try {
+ return store.get(versionedIdent, Entity.EntityType.FUNCTION,
FunctionEntity.class);
+
+} catch (NoSuchEntityException e) {
+ if (version == FunctionEntity.LATEST_VERSION) {
+throw new NoSuchFunctionException(e, "Function %s does not exist",
ident);
+ }
+ // Check if the function exists at all
+ try {
+store.get(
+toVersionedIdent(ident, FunctionEntity.LATEST_VERSION),
+Entity.EntityType.FUNCTION,
+FunctionEntity.class);
+// Function exists, but version doesn't
+throw new NoSuchFunctionVersionException(
+e, "Function %s version %d does not exist", ident, version);
+ } catch (NoSuchEntityException | IOException ex) {
+throw new NoSuchFunctionException(e, "Function %s does not exist",
ident);
+ }
+} catch (IOException e) {
+ throw new RuntimeException("Failed to get function " + ident, e);
+}
Review Comment:
Is it necessary to distinguish `NoSuchFunctionException` and
`NoSuchFunctionVersionException`, to handle this you need to call the DB twice,
which is not so efficient.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2672072902
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -71,29 +84,56 @@ public ManagedFunctionOperations(EntityStore store,
IdGenerator idGenerator) {
@Override
public NameIdentifier[] listFunctions(Namespace namespace) throws
NoSuchSchemaException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("listFunctions: FunctionEntity not
yet implemented");
+return Arrays.stream(listFunctionInfos(namespace))
+.map(f -> NameIdentifier.of(namespace, f.name()))
+.toArray(NameIdentifier[]::new);
}
@Override
public Function[] listFunctionInfos(Namespace namespace) throws
NoSuchSchemaException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"listFunctionInfos: FunctionEntity not yet implemented");
+try {
+ List functions =
+ store.list(namespace, FunctionEntity.class,
Entity.EntityType.FUNCTION);
+ return functions.toArray(FunctionEntity[]::new);
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
namespace);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to list functions in namespace " +
namespace, e);
+}
}
@Override
public Function getFunction(NameIdentifier ident) throws
NoSuchFunctionException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("getFunction: FunctionEntity not
yet implemented");
+return getFunction(ident, FunctionEntity.LATEST_VERSION);
}
@Override
public Function getFunction(NameIdentifier ident, int version)
Review Comment:
How do user know which versions they already have and which version to pick?
Since you don't have a list version interface.
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on PR #9560: URL: https://github.com/apache/gravitino/pull/9560#issuecomment-3723192907 > I suggest splitting this PR into two. You can move the `alterFunction` to another PR. done -- 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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
jerryshao commented on PR #9560: URL: https://github.com/apache/gravitino/pull/9560#issuecomment-3722852899 I suggest splitting this PR into two. You can move the `alterFunction` to another PR. -- 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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on PR #9560: URL: https://github.com/apache/gravitino/pull/9560#issuecomment-3722442012 @jerryshao could you plz help to review this PR when you have time, thx! -- 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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2671167546
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -117,21 +159,448 @@ public Function registerFunction(
FunctionColumn[] returnColumns,
FunctionDefinition[] definitions)
throws NoSuchSchemaException, FunctionAlreadyExistsException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"registerFunction for table-valued functions: FunctionEntity not yet
implemented");
+return doRegisterFunction(
+ident, comment, FunctionType.TABLE, deterministic, null,
returnColumns, definitions);
}
@Override
public Function alterFunction(NameIdentifier ident, FunctionChange...
changes)
throws NoSuchFunctionException, IllegalArgumentException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("alterFunction: FunctionEntity not
yet implemented");
+try {
+ return store.update(
+ ident,
+ FunctionEntity.class,
+ Entity.EntityType.FUNCTION,
+ oldEntity -> applyChanges(oldEntity, changes));
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchFunctionException(e, "Function %s does not exist",
ident);
+} catch (EntityAlreadyExistsException e) {
+ throw new IllegalArgumentException("Failed to alter function " + ident,
e);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to alter function " + ident, e);
+}
}
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ /**
+ * Converts a function identifier to a versioned identifier. The versioned
identifier uses the
+ * version number as the name to allow the store to retrieve specific
versions.
+ *
+ * @param ident The function identifier.
+ * @param version The version number, or {@link
FunctionEntity#LATEST_VERSION} for the latest.
+ * @return The versioned identifier.
+ */
+ private NameIdentifier toVersionedIdent(NameIdentifier ident, int version) {
+return NameIdentifier.of(
+ident.namespace().level(0),
+ident.namespace().level(1),
+ident.namespace().level(2),
+ident.name(),
+String.valueOf(version));
+ }
+
+ private Function doRegisterFunction(
+ NameIdentifier ident,
+ String comment,
+ FunctionType functionType,
+ boolean deterministic,
+ Type returnType,
+ FunctionColumn[] returnColumns,
+ FunctionDefinition[] definitions)
+ throws NoSuchSchemaException, FunctionAlreadyExistsException {
+Preconditions.checkArgument(
+definitions != null && definitions.length > 0,
+"At least one function definition must be provided");
+
+// Validate definitions for arity overlap when there are multiple
definitions
+if (definitions.length > 1) {
+ validateDefinitionsNoArityOverlap(definitions);
+}
+
+String currentUser = PrincipalUtils.getCurrentUserName();
+Instant now = Instant.now();
+AuditInfo auditInfo =
AuditInfo.builder().withCreator(currentUser).withCreateTime(now).build();
+
+FunctionEntity functionEntity =
+FunctionEntity.builder()
+.withId(idGenerator.nextId())
+.withName(ident.name())
+.withNamespace(ident.namespace())
+.withComment(comment)
+.withFunctionType(functionType)
+.withDeterministic(deterministic)
+.withReturnType(returnType)
+.withReturnColumns(returnColumns)
+.withDefinitions(definitions)
+.withVersion(INIT_VERSION)
+.withAuditInfo(auditInfo)
+.build();
+
+try {
+ store.put(functionEntity, false /* overwrite */);
+ return functionEntity;
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchSchemaException(e, "Schema %s does not exist",
ident.namespace());
+} catch (EntityAlreadyExistsException e) {
+ throw new FunctionAlreadyExistsException(e, "Function %s already
exists", ident);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to register function " + ident, e);
+}
+ }
+
+ private FunctionEntity applyChanges(FunctionEntity oldEntity,
FunctionChange... changes) {
+String newComment = oldEntity.comment();
+List newDefinitions =
+new ArrayList<>(Arrays.asList(oldEntity.definitions()));
+
+for (FunctionChange change : changes) {
+
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2667830594
##
core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java:
##
@@ -0,0 +1,357 @@
+/*
+ * 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.meta;
+
+import com.google.common.collect.Maps;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Objects;
+import lombok.ToString;
+import org.apache.gravitino.Auditable;
+import org.apache.gravitino.Entity;
+import org.apache.gravitino.Field;
+import org.apache.gravitino.HasIdentifier;
+import org.apache.gravitino.Namespace;
+import org.apache.gravitino.function.Function;
+import org.apache.gravitino.function.FunctionColumn;
+import org.apache.gravitino.function.FunctionDefinition;
+import org.apache.gravitino.function.FunctionType;
+import org.apache.gravitino.rel.types.Type;
+
+/**
+ * A class representing a function entity in the metadata store.
+ *
+ * This entity stores both the function metadata and its version
information together, avoiding
+ * the need for separate FunctionEntity and FunctionVersionEntity. When
retrieving, if version is
+ * set to the special value {@link #LATEST_VERSION}, the store should return
the latest version.
+ */
+@ToString
+public class FunctionEntity implements Entity, Auditable, HasIdentifier,
Function {
+
+ /** Special version value indicating the latest version should be retrieved.
*/
+ public static final int LATEST_VERSION = -1;
+
+ public static final Field ID =
+ Field.required("id", Long.class, "The unique id of the function
entity.");
+ public static final Field NAME =
+ Field.required("name", String.class, "The name of the function entity.");
+ public static final Field COMMENT =
+ Field.optional("comment", String.class, "The comment or description of
the function entity.");
+ public static final Field FUNCTION_TYPE =
+ Field.required("function_type", FunctionType.class, "The type of the
function.");
+ public static final Field DETERMINISTIC =
+ Field.required("deterministic", Boolean.class, "Whether the function is
deterministic.");
+ public static final Field RETURN_TYPE =
+ Field.optional(
+ "return_type", Type.class, "The return type for scalar or aggregate
functions.");
+ public static final Field RETURN_COLUMNS =
+ Field.optional(
+ "return_columns",
+ FunctionColumn[].class,
+ "The output columns for table-valued functions.");
+ public static final Field DEFINITIONS =
+ Field.required("definitions", FunctionDefinition[].class, "The
definitions of the function.");
+ public static final Field VERSION =
+ Field.required("version", Integer.class, "The version of the function
entity.");
+ public static final Field AUDIT_INFO =
+ Field.required("audit_info", AuditInfo.class, "The audit details of the
function entity.");
+
+ private Long id;
+ private String name;
+ private Namespace namespace;
+ private String comment;
+ private FunctionType functionType;
+ private boolean deterministic;
+ private Type returnType;
+ private FunctionColumn[] returnColumns;
+ private FunctionDefinition[] definitions;
+ private Integer version;
+ private AuditInfo auditInfo;
+
+ private FunctionEntity() {}
+
+ @Override
+ public Map fields() {
+Map fields = Maps.newHashMap();
+fields.put(ID, id);
+fields.put(NAME, name);
+fields.put(COMMENT, comment);
+fields.put(FUNCTION_TYPE, functionType);
+fields.put(DETERMINISTIC, deterministic);
+fields.put(RETURN_TYPE, returnType);
+fields.put(RETURN_COLUMNS, returnColumns);
+fields.put(DEFINITIONS, definitions);
+fields.put(VERSION, version);
+fields.put(AUDIT_INFO, auditInfo);
+
+return Collections.unmodifiableMap(fields);
+ }
+
+ @Override
+ public String name() {
+return name;
+ }
+
+ @Override
+ public Long id() {
+return id;
+ }
+
+ @Override
+ public Namespace namespace() {
+return namespace;
+ }
+
+ @Override
+ public String comment() {
+return comment;
+ }
+
+ @Override
+ public FunctionType functionType() {
+return funct
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
mchades commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2667786393
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -117,21 +159,448 @@ public Function registerFunction(
FunctionColumn[] returnColumns,
FunctionDefinition[] definitions)
throws NoSuchSchemaException, FunctionAlreadyExistsException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"registerFunction for table-valued functions: FunctionEntity not yet
implemented");
+return doRegisterFunction(
+ident, comment, FunctionType.TABLE, deterministic, null,
returnColumns, definitions);
}
@Override
public Function alterFunction(NameIdentifier ident, FunctionChange...
changes)
throws NoSuchFunctionException, IllegalArgumentException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("alterFunction: FunctionEntity not
yet implemented");
+try {
+ return store.update(
+ ident,
+ FunctionEntity.class,
+ Entity.EntityType.FUNCTION,
+ oldEntity -> applyChanges(oldEntity, changes));
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchFunctionException(e, "Function %s does not exist",
ident);
+} catch (EntityAlreadyExistsException e) {
+ throw new IllegalArgumentException("Failed to alter function " + ident,
e);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to alter function " + ident, e);
+}
}
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ /**
+ * Converts a function identifier to a versioned identifier. The versioned
identifier uses the
+ * version number as the name to allow the store to retrieve specific
versions.
+ *
+ * @param ident The function identifier.
+ * @param version The version number, or {@link
FunctionEntity#LATEST_VERSION} for the latest.
+ * @return The versioned identifier.
+ */
+ private NameIdentifier toVersionedIdent(NameIdentifier ident, int version) {
+return NameIdentifier.of(
+ident.namespace().level(0),
+ident.namespace().level(1),
+ident.namespace().level(2),
+ident.name(),
+String.valueOf(version));
Review Comment:
The storage will do the check in the next PR
--
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]
Re: [PR] [#9561] feat(core): Add UDF management core operations and validations [gravitino]
Copilot commented on code in PR #9560:
URL: https://github.com/apache/gravitino/pull/9560#discussion_r2667207559
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -117,21 +159,448 @@ public Function registerFunction(
FunctionColumn[] returnColumns,
FunctionDefinition[] definitions)
throws NoSuchSchemaException, FunctionAlreadyExistsException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"registerFunction for table-valued functions: FunctionEntity not yet
implemented");
+return doRegisterFunction(
+ident, comment, FunctionType.TABLE, deterministic, null,
returnColumns, definitions);
}
@Override
public Function alterFunction(NameIdentifier ident, FunctionChange...
changes)
throws NoSuchFunctionException, IllegalArgumentException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("alterFunction: FunctionEntity not
yet implemented");
+try {
+ return store.update(
+ ident,
+ FunctionEntity.class,
+ Entity.EntityType.FUNCTION,
+ oldEntity -> applyChanges(oldEntity, changes));
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchFunctionException(e, "Function %s does not exist",
ident);
+} catch (EntityAlreadyExistsException e) {
+ throw new IllegalArgumentException("Failed to alter function " + ident,
e);
+} catch (IOException e) {
+ throw new RuntimeException("Failed to alter function " + ident, e);
+}
}
@Override
public boolean dropFunction(NameIdentifier ident) {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("dropFunction: FunctionEntity not
yet implemented");
+try {
+ return store.delete(ident, Entity.EntityType.FUNCTION);
+} catch (NoSuchEntityException e) {
+ return false;
+} catch (IOException e) {
+ throw new RuntimeException("Failed to drop function " + ident, e);
+}
+ }
+
+ /**
+ * Converts a function identifier to a versioned identifier. The versioned
identifier uses the
+ * version number as the name to allow the store to retrieve specific
versions.
+ *
+ * @param ident The function identifier.
+ * @param version The version number, or {@link
FunctionEntity#LATEST_VERSION} for the latest.
+ * @return The versioned identifier.
+ */
+ private NameIdentifier toVersionedIdent(NameIdentifier ident, int version) {
+return NameIdentifier.of(
+ident.namespace().level(0),
+ident.namespace().level(1),
+ident.namespace().level(2),
+ident.name(),
+String.valueOf(version));
Review Comment:
The toVersionedIdent method hardcodes namespace level access (level(0),
level(1), level(2)), which assumes the namespace always has exactly 3 levels.
This is fragile and could cause ArrayIndexOutOfBoundsException if used with a
namespace that has fewer than 3 levels. Consider using namespace.levels() to
construct the versioned identifier more robustly, or add a precondition check
to validate the namespace length.
```suggestion
String[] namespaceLevels = ident.namespace().levels();
String[] versionedNamespaceLevels = Arrays.copyOf(namespaceLevels,
namespaceLevels.length + 1);
versionedNamespaceLevels[namespaceLevels.length] = ident.name();
return NameIdentifier.of(versionedNamespaceLevels,
String.valueOf(version));
```
##
core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java:
##
@@ -117,21 +159,448 @@ public Function registerFunction(
FunctionColumn[] returnColumns,
FunctionDefinition[] definitions)
throws NoSuchSchemaException, FunctionAlreadyExistsException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException(
-"registerFunction for table-valued functions: FunctionEntity not yet
implemented");
+return doRegisterFunction(
+ident, comment, FunctionType.TABLE, deterministic, null,
returnColumns, definitions);
}
@Override
public Function alterFunction(NameIdentifier ident, FunctionChange...
changes)
throws NoSuchFunctionException, IllegalArgumentException {
-// TODO: Implement when FunctionEntity is available
-throw new UnsupportedOperationException("alterFunction: FunctionEntity not
yet implemented");
+try {
+ return store.update(
+ ident,
+ FunctionEntity.class,
+ Entity.EntityType.FUNCTION,
+ oldEntity -> applyChanges(oldEntity, changes));
+
+} catch (NoSuchEntityException e) {
+ throw new NoSuchFunctionException(e, "Function %s does not exist",
ident);
+} catch (EntityAlreadyExistsException e) {
+ throw new IllegalArgumentException("Failed to alter function " + ident,
e);
+} catch
