This is an automated email from the ASF dual-hosted git repository. mchades pushed a commit to branch fix/improve-updateImpl-error-message in repository https://gitbox.apache.org/repos/asf/gravitino.git
commit 0abed1bb30123944a7eb381dfc0f30cd67ed3168 Author: mchades <[email protected]> AuthorDate: Thu Feb 26 13:17:57 2026 +0800 Improve error messages for updateImpl/removeImpl/addImpl operations When operations fail due to parameter or runtime mismatches, the error messages now include actionable context to help users diagnose the issue: - 'no definition found': lists available parameter signatures so users can verify the correct format (e.g., 'integer' vs 'int') - 'runtime not found': lists available runtimes in the matched definition so users can distinguish a missing runtime from a parameter mismatch Fixes #10002 Co-authored-by: Copilot <[email protected]> --- .../catalog/ManagedFunctionOperations.java | 46 ++++++++- .../catalog/TestManagedFunctionOperations.java | 106 ++++++++++++++++++--- 2 files changed, 136 insertions(+), 16 deletions(-) diff --git a/core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java b/core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java index 7341052cce..30418424ba 100644 --- a/core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java +++ b/core/src/main/java/org/apache/gravitino/catalog/ManagedFunctionOperations.java @@ -451,6 +451,19 @@ public class ManagedFunctionOperations implements FunctionCatalog { } } + private static String formatParams(FunctionParam[] params) { + if (params == null || params.length == 0) { + return "()"; + } + StringBuilder sb = new StringBuilder("("); + for (int i = 0; i < params.length; i++) { + if (i > 0) sb.append(", "); + sb.append(params[i].name()).append(": ").append(params[i].dataType().simpleString()); + } + sb.append(")"); + return sb.toString(); + } + private boolean parametersMatch(FunctionParam[] params1, FunctionParam[] params2) { if (params1.length != params2.length) { return false; @@ -491,8 +504,13 @@ public class ManagedFunctionOperations implements FunctionCatalog { } if (!found) { + List<String> availableSignatures = + definitions.stream().map(d -> formatParams(d.parameters())).collect(Collectors.toList()); throw new IllegalArgumentException( - "Cannot add implementation: no definition found with the specified parameters"); + String.format( + "Cannot add implementation: no definition found with the specified parameters %s. " + + "Available parameter signatures: %s", + formatParams(targetParams), availableSignatures)); } return result; @@ -506,12 +524,14 @@ public class ManagedFunctionOperations implements FunctionCatalog { List<FunctionDefinition> result = new ArrayList<>(); boolean definitionFound = false; boolean runtimeFound = false; + List<FunctionImpl.RuntimeType> availableRuntimes = new ArrayList<>(); for (FunctionDefinition def : definitions) { if (parametersMatch(def.parameters(), targetParams)) { definitionFound = true; List<FunctionImpl> impls = new ArrayList<>(); for (FunctionImpl impl : def.impls()) { + availableRuntimes.add(impl.runtime()); if (impl.runtime() == runtime) { runtimeFound = true; impls.add(newImpl); @@ -526,14 +546,21 @@ public class ManagedFunctionOperations implements FunctionCatalog { } if (!definitionFound) { + List<String> availableSignatures = + definitions.stream().map(d -> formatParams(d.parameters())).collect(Collectors.toList()); throw new IllegalArgumentException( - "Cannot update implementation: no definition found with the specified parameters"); + String.format( + "Cannot update implementation: no definition found with the specified parameters %s. " + + "Available parameter signatures: %s", + formatParams(targetParams), availableSignatures)); } if (!runtimeFound) { throw new IllegalArgumentException( String.format( - "Cannot update implementation: runtime '%s' not found in the definition", runtime)); + "Cannot update implementation: runtime '%s' not found in the definition %s. " + + "Available runtimes in that definition: %s", + runtime, formatParams(targetParams), availableRuntimes)); } return result; @@ -546,6 +573,7 @@ public class ManagedFunctionOperations implements FunctionCatalog { List<FunctionDefinition> result = new ArrayList<>(); boolean definitionFound = false; boolean runtimeFound = false; + List<FunctionImpl.RuntimeType> availableRuntimes = new ArrayList<>(); for (FunctionDefinition def : definitions) { if (parametersMatch(def.parameters(), targetParams)) { @@ -561,6 +589,7 @@ public class ManagedFunctionOperations implements FunctionCatalog { List<FunctionImpl> impls = new ArrayList<>(); for (FunctionImpl impl : def.impls()) { + availableRuntimes.add(impl.runtime()); if (impl.runtime() == runtime) { runtimeFound = true; } else { @@ -574,14 +603,21 @@ public class ManagedFunctionOperations implements FunctionCatalog { } if (!definitionFound) { + List<String> availableSignatures = + definitions.stream().map(d -> formatParams(d.parameters())).collect(Collectors.toList()); throw new IllegalArgumentException( - "Cannot remove implementation: no definition found with the specified parameters"); + String.format( + "Cannot remove implementation: no definition found with the specified parameters %s. " + + "Available parameter signatures: %s", + formatParams(targetParams), availableSignatures)); } if (!runtimeFound) { throw new IllegalArgumentException( String.format( - "Cannot remove implementation: runtime '%s' not found in the definition", runtime)); + "Cannot remove implementation: runtime '%s' not found in the definition %s. " + + "Available runtimes in that definition: %s", + runtime, formatParams(targetParams), availableRuntimes)); } return result; diff --git a/core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java b/core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java index ff3b6638e2..60d5513f99 100644 --- a/core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java +++ b/core/src/test/java/org/apache/gravitino/catalog/TestManagedFunctionOperations.java @@ -627,12 +627,54 @@ public class TestManagedFunctionOperations { FunctionImpl trinoImpl = FunctionImpls.ofJava(FunctionImpl.RuntimeType.TRINO, "com.example.TrinoUDF"); - Assertions.assertThrows( - IllegalArgumentException.class, - () -> - functionOperations.alterFunction( - funcIdent, - FunctionChange.updateImpl(params, FunctionImpl.RuntimeType.TRINO, trinoImpl))); + IllegalArgumentException ex = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + functionOperations.alterFunction( + funcIdent, + FunctionChange.updateImpl(params, FunctionImpl.RuntimeType.TRINO, trinoImpl))); + Assertions.assertTrue( + ex.getMessage().contains("TRINO"), "Error message should mention the missing runtime"); + Assertions.assertTrue( + ex.getMessage().contains("SPARK"), + "Error message should list available runtimes in that definition"); + } + + @Test + public void testAlterFunctionUpdateImplWrongParameters() { + NameIdentifier funcIdent = getFunctionIdent("func_update_impl_wrongparams"); + FunctionParam[] params = new FunctionParam[] {FunctionParams.of("a", Types.IntegerType.get())}; + FunctionImpl sparkImpl = + FunctionImpls.ofJava(FunctionImpl.RuntimeType.SPARK, "com.example.SparkUDF"); + FunctionDefinition[] definitions = + new FunctionDefinition[] { + createDefinitionWithImpls(params, Types.StringType.get(), new FunctionImpl[] {sparkImpl}) + }; + + functionOperations.registerFunction( + funcIdent, "Test function", FunctionType.SCALAR, true, definitions); + + // Try to update with wrong parameter name - definition won't be found + FunctionParam[] wrongParams = + new FunctionParam[] {FunctionParams.of("x", Types.IntegerType.get())}; + FunctionImpl newImpl = + FunctionImpls.ofJava(FunctionImpl.RuntimeType.SPARK, "com.example.NewSparkUDF"); + + IllegalArgumentException ex = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + functionOperations.alterFunction( + funcIdent, + FunctionChange.updateImpl( + wrongParams, FunctionImpl.RuntimeType.SPARK, newImpl))); + Assertions.assertTrue( + ex.getMessage().contains("(x: integer)"), + "Error message should include the specified parameter signature"); + Assertions.assertTrue( + ex.getMessage().contains("(a: integer)"), + "Error message should list available parameter signatures"); } @Test @@ -699,11 +741,53 @@ public class TestManagedFunctionOperations { funcIdent, "Test function", FunctionType.SCALAR, true, definitions); // Try to remove Trino implementation which doesn't exist - Assertions.assertThrows( - IllegalArgumentException.class, - () -> - functionOperations.alterFunction( - funcIdent, FunctionChange.removeImpl(params, FunctionImpl.RuntimeType.TRINO))); + IllegalArgumentException ex = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + functionOperations.alterFunction( + funcIdent, FunctionChange.removeImpl(params, FunctionImpl.RuntimeType.TRINO))); + Assertions.assertTrue( + ex.getMessage().contains("TRINO"), "Error message should mention the missing runtime"); + Assertions.assertTrue( + ex.getMessage().contains("SPARK"), + "Error message should list available runtimes in that definition"); + } + + @Test + public void testAlterFunctionRemoveImplWrongParameters() { + NameIdentifier funcIdent = getFunctionIdent("func_remove_impl_wrongparams"); + FunctionParam[] params = new FunctionParam[] {FunctionParams.of("a", Types.IntegerType.get())}; + FunctionImpl sparkImpl = + FunctionImpls.ofJava(FunctionImpl.RuntimeType.SPARK, "com.example.SparkUDF"); + FunctionImpl trinoImpl = + FunctionImpls.ofJava(FunctionImpl.RuntimeType.TRINO, "com.example.TrinoUDF"); + FunctionDefinition[] definitions = + new FunctionDefinition[] { + createDefinitionWithImpls( + params, Types.StringType.get(), new FunctionImpl[] {sparkImpl, trinoImpl}) + }; + + functionOperations.registerFunction( + funcIdent, "Test function", FunctionType.SCALAR, true, definitions); + + // Try to remove with wrong parameter type - definition won't be found + FunctionParam[] wrongParams = + new FunctionParam[] {FunctionParams.of("a", Types.LongType.get())}; + + IllegalArgumentException ex = + Assertions.assertThrows( + IllegalArgumentException.class, + () -> + functionOperations.alterFunction( + funcIdent, + FunctionChange.removeImpl(wrongParams, FunctionImpl.RuntimeType.SPARK))); + Assertions.assertTrue( + ex.getMessage().contains("(a: long)"), + "Error message should include the specified parameter signature"); + Assertions.assertTrue( + ex.getMessage().contains("(a: integer)"), + "Error message should list available parameter signatures"); } @Test
