[#9561] feat(core): Add UDF management core operations and validations #9560

mchades · 2025-12-25T12:11:14Z

What changes were proposed in this pull request?

This PR implements the core operations for UDF (User-Defined Function) management in Gravitino, including:

ManagedFunctionOperations implementation:

registerFunction: Register scalar, aggregate, and table functions with multi-definition support
getFunction: Retrieve function by identifier (latest version)
listFunctions: List all functions in a schema namespace
dropFunction: Remove a function

Unit tests:

Added comprehensive unit tests for ManagedFunctionOperations with test cases covering CRUD operations (except alter) and validation scenarios

Why are the changes needed?

Fix: #9561

This is part of the UDF management feature for Gravitino. The core operations layer provides the business logic for function management, ensuring data integrity through proper validations.

Does this PR introduce any user-facing change?

No, this is an internal implementation.

How was this patch tested?

Added unit tests in TestManagedFunctionOperations.java covering:

Function registration and listing
Function retrieval (get)
Function deletion (drop)
Error cases for invalid operations

Copilot

Pull request overview

This PR implements core operations for UDF (User-Defined Function) management in Apache Gravitino, providing the foundation for managing user-defined functions within the catalog system. The implementation includes comprehensive validation logic to prevent ambiguous function invocations through arity overlap detection.

Key changes:

Implements ManagedFunctionOperations with full CRUD operations (register, get, list, alter, drop) for UDF management
Adds FunctionEntity as the metadata storage representation of functions with versioning support
Implements sophisticated validation for FunctionChange operations, including arity overlap detection for function definitions with default parameters

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java`	Core implementation of function catalog operations including registration, retrieval, alteration, and deletion with validation logic
`core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java`	Entity class representing function metadata with builder pattern and support for versioning
`core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java`	Comprehensive unit tests covering all CRUD operations, function change validations, and edge cases
`core/src/main/java/org/apache/gravitino/Entity.java`	Adds FUNCTION to the EntityType enum
`core/src/main/java/org/apache/gravitino/storage/relational/JDBCBackend.java`	Adds FUNCTION entity type handling to garbage collection methods with TODO note for future versioning support

core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java

core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java

mchades · 2026-01-08T07:20:10Z

@jerryshao could you plz help to review this PR when you have time, thx!

jerryshao · 2026-01-08T08:51:45Z

I suggest splitting this PR into two. You can move the alterFunction to another PR.

mchades · 2026-01-08T10:21:42Z

I suggest splitting this PR into two. You can move the alterFunction to another PR.

done

jerryshao · 2026-01-08T11:58:34Z

core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java

  }

  @Override
  public Function getFunction(NameIdentifier ident, int version)


How do user know which versions they already have and which version to pick? Since you don't have a list version interface.

jerryshao · 2026-01-08T12:01:24Z

core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java

+      }
+    } catch (IOException e) {
+      throw new RuntimeException("Failed to get function " + ident, e);
+    }


Is it necessary to distinguish NoSuchFunctionException and NoSuchFunctionVersionException, to handle this you need to call the DB twice, which is not so efficient.

jerryshao · 2026-01-08T12:04:45Z

core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java

-    throw new UnsupportedOperationException(
-        "registerFunction for table-valued functions: FunctionEntity not yet implemented");
+    return doRegisterFunction(
+        ident, comment, FunctionType.TABLE, deterministic, null, returnColumns, definitions);


I'm inclined to use Optional instead of null, which has no type information.

jerryshao · 2026-01-08T12:11:14Z

core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java

-    // TODO: Implement when FunctionEntity is available
-    throw new UnsupportedOperationException("dropFunction: FunctionEntity not yet implemented");
+    try {
+      return store.delete(ident, Entity.EntityType.FUNCTION);


Will this drop all the versions?

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.

jerryshao · 2026-01-08T12:18:52Z

core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java

+   * @param definitions The array of definitions to validate.
+   * @throws IllegalArgumentException If any two definitions have overlapping arities.
+   */
+  private void validateDefinitionsNoArityOverlap(FunctionDefinition[] definitions) {


I would suggest you to add more tests for this method.

jerryshao · 2026-01-08T12:28:32Z

core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java

+   *   <li>{@code foo(int a, float b)} → arities: {@code ["int,float"]}
+   *   <li>{@code foo(int a, float b default 1.0)} → arities: {@code ["int", "int,float"]}
+   *   <li>{@code foo(int a, float b default 1.0, string c default 'x')} → arities: {@code ["int",
+   *       "int,float", "int,float,string"]}


Can it be "int,string"?

No, "int,string" is not a valid arity here. Since the current arity design is based on positional parameter matching, you cannot skip the middle parameter b to directly pass c.

For engines that support named parameters (like Python), the parameter names and values would be passed together at invocation time (e.g., {a: 1, c: "hello"}). In that case, the engine can resolve the correct definition by mapping parameter names to their positions and validating that any missing parameters have default values. This would be handled at runtime by the engine, not during function registration.

jerryshao · 2026-01-08T12:36:04Z

core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java

+    // Find the first parameter with a default value
+    int firstDefaultIndex = params.length;
+    for (int i = 0; i < params.length; i++) {
+      if (params[i].defaultValue() != Column.DEFAULT_VALUE_NOT_SET) {
+        firstDefaultIndex = i;
+        break;
+      }
+    }


I think this loop can be merged with above.

jerryshao · 2026-01-08T12:49:36Z

core/src/main/java/org/apache/gravitino/meta/FunctionEntity.java

+  public static final Field VERSION =
+      Field.required("version", Integer.class, "The version of the function entity.");


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?

jerryshao · 2026-01-08T12:55:53Z

core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java

+        new FunctionParam[] {FunctionParams.of("input", Types.StringType.get())};
+    FunctionDefinition[] definitions = new FunctionDefinition[] {createSimpleDefinition(params)};
+
+    org.apache.gravitino.function.Function newFunc =


Don't use full qualified package name.

jerryshao · 2026-01-08T12:57:13Z

core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java

+  }
+
+  @Test
+  public void testInvalidParameterOrder() {


Move all the public test methods above the private methods.

mchades marked this pull request as draft December 25, 2025 12:12

mchades changed the title ~~feat(core): Add UDF management core operations and validations~~ [#9561] feat(core): Add UDF management core operations and validations Dec 25, 2025

mchades force-pushed the pr-udf-core branch 2 times, most recently from 5599a23 to 3c47d32 Compare December 30, 2025 14:26

add core udf operations

8015f2a

mchades force-pushed the pr-udf-core branch from 3c47d32 to 8015f2a Compare January 6, 2026 08:32

mchades marked this pull request as ready for review January 6, 2026 08:32

mchades requested a review from jerryshao January 6, 2026 08:32

jerryshao assigned mchades Jan 7, 2026

jerryshao requested a review from Copilot January 7, 2026 06:09

Copilot started reviewing on behalf of jerryshao January 7, 2026 06:09 View session

Copilot AI reviewed Jan 7, 2026

View reviewed changes

validate default position

7e7b831

mchades force-pushed the pr-udf-core branch from d752536 to 7e7b831 Compare January 7, 2026 15:54

mchades force-pushed the pr-udf-core branch from caad7e0 to fd94479 Compare January 8, 2026 10:22

remove alter ops code to split PR

60cb5d7

mchades force-pushed the pr-udf-core branch from fd94479 to 60cb5d7 Compare January 8, 2026 10:24

jerryshao reviewed Jan 8, 2026

View reviewed changes

		public static final Field VERSION =
		Field.required("version", Integer.class, "The version of the function entity.");

[#9561] feat(core): Add UDF management core operations and validations #9560

Are you sure you want to change the base?

[#9561] feat(core): Add UDF management core operations and validations #9560

Conversation

mchades commented Dec 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mchades commented Jan 8, 2026

Uh oh!

jerryshao commented Jan 8, 2026

Uh oh!

mchades commented Jan 8, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mchades commented Dec 25, 2025 •

edited

Loading