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.
+   *
+   * <p>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<String> aritiesI = computeArities(definitions[i]);
+      for (int j = i + 1; j < definitions.length; j++) {
+        Set<String> 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 function definition. A 
definition with N
+   * parameters where the last M have default values supports arities from 
(N-M) to N parameters.
+   *
+   * <p>For example:
+   *
+   * <ul>
+   *   <li>{@code foo(int a)} → arities: {@code ["int"]}
+   *   <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"]}

Review Comment:
   Can it be "int,string"?



-- 
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]

Reply via email to