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<Type> returnType,
+ Optional<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.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.
+ *
+ * <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));
+ }
+ }
+ }
+ }
+ }
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]
