github-actions[bot] commented on code in PR #65100:
URL: https://github.com/apache/doris/pull/65100#discussion_r3510452288


##########
fe/fe-core/src/test/java/org/apache/doris/catalog/CreateFunctionTest.java:
##########
@@ -170,6 +176,151 @@ public void testCreatePythonFunctionRejectsObjectTypes() 
throws Exception {
                 "ARRAY unsupported sub-type: bitmap");
     }
 
+    @Test
+    public void testCreateFunctionRollbackOnlyFailedOverload() throws 
Exception {
+        ConnectContext ctx = UtFrameUtils.createDefaultCtx();
+        createDatabase(ctx, "create database rollback_function_db;");
+        Database db = 
Env.getCurrentInternalCatalog().getDbNullable("rollback_function_db");
+        Assert.assertNotNull(db);
+
+        EditLog editLog = Env.getCurrentEnv().getEditLog();
+        EditLog spyEditLog = Mockito.spy(editLog);
+        
Mockito.doNothing().when(spyEditLog).logAddFunction(Mockito.any(Function.class));
+        Env.getCurrentEnv().setEditLog(spyEditLog);
+        try (MockedStatic<FunctionUtil> mockedFunctionUtil = 
Mockito.mockStatic(FunctionUtil.class,
+                Mockito.CALLS_REAL_METHODS)) {
+            Function existingFunction = createJavaUdf("rollback_function_db", 
"rollback_fn", Type.INT);
+            db.addFunction(existingFunction, false);
+            Assert.assertSame(existingFunction, 
db.getFunction(searchDesc(existingFunction)));
+
+            Mockito.clearInvocations(spyEditLog);
+            Function failedFunction = createJavaUdf("rollback_function_db", 
"rollback_fn", Type.BIGINT);
+            mockedFunctionUtil.when(() -> 
FunctionUtil.translateToNereidsThrows(
+                    "rollback_function_db", failedFunction)).thenThrow(new 
RuntimeException("translate failed"));
+
+            RuntimeException exception = 
Assert.assertThrows(RuntimeException.class,
+                    () -> db.addFunction(failedFunction, false));
+            Assert.assertEquals("translate failed", exception.getMessage());
+            Mockito.verify(spyEditLog, 
Mockito.never()).logAddFunction(Mockito.any(Function.class));
+            Assert.assertSame(existingFunction, 
db.getFunction(searchDesc(existingFunction)));
+            Assert.assertThrows(AnalysisException.class, () -> 
db.getFunction(searchDesc(failedFunction)));
+        } finally {
+            Env.getCurrentEnv().setEditLog(editLog);
+        }
+    }
+
+    @Test
+    public void testCreateTableFunctionRollbackWhenOuterFunctionFails() throws 
Exception {
+        ConnectContext ctx = UtFrameUtils.createDefaultCtx();
+        createDatabase(ctx, "create database rollback_table_function_db;");
+        Database db = 
Env.getCurrentInternalCatalog().getDbNullable("rollback_table_function_db");
+        Assert.assertNotNull(db);
+
+        EditLog editLog = Env.getCurrentEnv().getEditLog();
+        EditLog spyEditLog = Mockito.spy(editLog);
+        
Mockito.doNothing().when(spyEditLog).logAddFunction(Mockito.any(Function.class));
+        Env.getCurrentEnv().setEditLog(spyEditLog);
+        try (MockedStatic<FunctionUtil> mockedFunctionUtil = 
Mockito.mockStatic(FunctionUtil.class,
+                Mockito.CALLS_REAL_METHODS)) {
+            Function tableFunction = 
createJavaUdtf("rollback_table_function_db", "rollback_table_fn", Type.INT);
+            mockedFunctionUtil.when(() -> 
FunctionUtil.translateToNereidsThrows(
+                    Mockito.eq("rollback_table_function_db"), 
Mockito.any(Function.class)))
+                    .thenAnswer(invocation -> {
+                        Function function = invocation.getArgument(1);
+                        if 
("rollback_table_fn_outer".equals(function.functionName())) {
+                            throw new RuntimeException("outer translate 
failed");
+                        }
+                        return true;
+                    });
+
+            RuntimeException exception = 
Assert.assertThrows(RuntimeException.class,
+                    () -> db.addTableFunction(tableFunction, false));
+            Assert.assertEquals("outer translate failed", 
exception.getMessage());
+            Mockito.verify(spyEditLog, 
Mockito.never()).logAddFunction(Mockito.any(Function.class));
+            Assert.assertThrows(AnalysisException.class, () -> 
db.getFunction(searchDesc(tableFunction)));
+            Assert.assertThrows(AnalysisException.class,
+                    () -> 
db.getFunction(searchDesc("rollback_table_function_db", 
"rollback_table_fn_outer",
+                            Type.INT)));
+            mockedFunctionUtil.verify(() -> 
FunctionUtil.dropFromNereids(Mockito.eq("rollback_table_function_db"),
+                    Mockito.argThat(function -> 
"rollback_table_fn".equals(function.getName().getFunction()))));
+            mockedFunctionUtil.verify(() -> 
FunctionUtil.dropFromNereids(Mockito.eq("rollback_table_function_db"),
+                    Mockito.argThat(function -> 
"rollback_table_fn_outer".equals(function.getName().getFunction()))));
+        } finally {
+            Env.getCurrentEnv().setEditLog(editLog);
+        }
+    }

Review Comment:
   This assertion will not match the `UserException` thrown by the conflict 
path. `FunctionUtil.addFunctionImpl()` throws `new UserException("function 
already exists")`, but `UserException.getMessage()` formats it as `errCode = 2, 
detailMessage = function already exists`; the bare text is returned by 
`getDetailMessage()`. As written, this new test fails on the assertion instead 
of validating rollback. Please assert `exception.getDetailMessage()` here, or 
compare `getMessage()` against the full formatted string.



##########
fe/fe-core/src/test/java/org/apache/doris/catalog/CreateFunctionTest.java:
##########
@@ -170,6 +176,151 @@ public void testCreatePythonFunctionRejectsObjectTypes() 
throws Exception {
                 "ARRAY unsupported sub-type: bitmap");
     }
 
+    @Test
+    public void testCreateFunctionRollbackOnlyFailedOverload() throws 
Exception {
+        ConnectContext ctx = UtFrameUtils.createDefaultCtx();
+        createDatabase(ctx, "create database rollback_function_db;");
+        Database db = 
Env.getCurrentInternalCatalog().getDbNullable("rollback_function_db");
+        Assert.assertNotNull(db);
+
+        EditLog editLog = Env.getCurrentEnv().getEditLog();
+        EditLog spyEditLog = Mockito.spy(editLog);
+        
Mockito.doNothing().when(spyEditLog).logAddFunction(Mockito.any(Function.class));
+        Env.getCurrentEnv().setEditLog(spyEditLog);
+        try (MockedStatic<FunctionUtil> mockedFunctionUtil = 
Mockito.mockStatic(FunctionUtil.class,
+                Mockito.CALLS_REAL_METHODS)) {
+            Function existingFunction = createJavaUdf("rollback_function_db", 
"rollback_fn", Type.INT);
+            db.addFunction(existingFunction, false);
+            Assert.assertSame(existingFunction, 
db.getFunction(searchDesc(existingFunction)));
+
+            Mockito.clearInvocations(spyEditLog);
+            Function failedFunction = createJavaUdf("rollback_function_db", 
"rollback_fn", Type.BIGINT);
+            mockedFunctionUtil.when(() -> 
FunctionUtil.translateToNereidsThrows(
+                    "rollback_function_db", failedFunction)).thenThrow(new 
RuntimeException("translate failed"));
+
+            RuntimeException exception = 
Assert.assertThrows(RuntimeException.class,
+                    () -> db.addFunction(failedFunction, false));
+            Assert.assertEquals("translate failed", exception.getMessage());
+            Mockito.verify(spyEditLog, 
Mockito.never()).logAddFunction(Mockito.any(Function.class));
+            Assert.assertSame(existingFunction, 
db.getFunction(searchDesc(existingFunction)));
+            Assert.assertThrows(AnalysisException.class, () -> 
db.getFunction(searchDesc(failedFunction)));
+        } finally {
+            Env.getCurrentEnv().setEditLog(editLog);
+        }
+    }
+
+    @Test
+    public void testCreateTableFunctionRollbackWhenOuterFunctionFails() throws 
Exception {
+        ConnectContext ctx = UtFrameUtils.createDefaultCtx();
+        createDatabase(ctx, "create database rollback_table_function_db;");
+        Database db = 
Env.getCurrentInternalCatalog().getDbNullable("rollback_table_function_db");
+        Assert.assertNotNull(db);
+
+        EditLog editLog = Env.getCurrentEnv().getEditLog();
+        EditLog spyEditLog = Mockito.spy(editLog);
+        
Mockito.doNothing().when(spyEditLog).logAddFunction(Mockito.any(Function.class));
+        Env.getCurrentEnv().setEditLog(spyEditLog);
+        try (MockedStatic<FunctionUtil> mockedFunctionUtil = 
Mockito.mockStatic(FunctionUtil.class,
+                Mockito.CALLS_REAL_METHODS)) {
+            Function tableFunction = 
createJavaUdtf("rollback_table_function_db", "rollback_table_fn", Type.INT);
+            mockedFunctionUtil.when(() -> 
FunctionUtil.translateToNereidsThrows(
+                    Mockito.eq("rollback_table_function_db"), 
Mockito.any(Function.class)))
+                    .thenAnswer(invocation -> {
+                        Function function = invocation.getArgument(1);
+                        if 
("rollback_table_fn_outer".equals(function.functionName())) {
+                            throw new RuntimeException("outer translate 
failed");
+                        }
+                        return true;
+                    });
+
+            RuntimeException exception = 
Assert.assertThrows(RuntimeException.class,
+                    () -> db.addTableFunction(tableFunction, false));
+            Assert.assertEquals("outer translate failed", 
exception.getMessage());
+            Mockito.verify(spyEditLog, 
Mockito.never()).logAddFunction(Mockito.any(Function.class));
+            Assert.assertThrows(AnalysisException.class, () -> 
db.getFunction(searchDesc(tableFunction)));
+            Assert.assertThrows(AnalysisException.class,
+                    () -> 
db.getFunction(searchDesc("rollback_table_function_db", 
"rollback_table_fn_outer",
+                            Type.INT)));
+            mockedFunctionUtil.verify(() -> 
FunctionUtil.dropFromNereids(Mockito.eq("rollback_table_function_db"),
+                    Mockito.argThat(function -> 
"rollback_table_fn".equals(function.getName().getFunction()))));
+            mockedFunctionUtil.verify(() -> 
FunctionUtil.dropFromNereids(Mockito.eq("rollback_table_function_db"),
+                    Mockito.argThat(function -> 
"rollback_table_fn_outer".equals(function.getName().getFunction()))));
+        } finally {
+            Env.getCurrentEnv().setEditLog(editLog);
+        }
+    }
+
+    @Test
+    public void testCreateTableFunctionRollbackWhenOuterFunctionConflicts() 
throws Exception {
+        ConnectContext ctx = UtFrameUtils.createDefaultCtx();
+        createDatabase(ctx, "create database 
rollback_table_function_conflict_db;");
+        Database db = 
Env.getCurrentInternalCatalog().getDbNullable("rollback_table_function_conflict_db");
+        Assert.assertNotNull(db);
+
+        EditLog editLog = Env.getCurrentEnv().getEditLog();
+        EditLog spyEditLog = Mockito.spy(editLog);
+        
Mockito.doNothing().when(spyEditLog).logAddFunction(Mockito.any(Function.class));
+        Env.getCurrentEnv().setEditLog(spyEditLog);
+        try (MockedStatic<FunctionUtil> mockedFunctionUtil = 
Mockito.mockStatic(FunctionUtil.class,
+                Mockito.CALLS_REAL_METHODS)) {
+            mockedFunctionUtil.when(() -> 
FunctionUtil.translateToNereidsThrows(
+                    Mockito.eq("rollback_table_function_conflict_db"), 
Mockito.any(Function.class)))
+                    .thenReturn(true);
+            Function existingOuterFunction = createJavaUdtf(
+                    "rollback_table_function_conflict_db", 
"rollback_table_conflict_fn_outer", Type.INT);
+            db.addFunction(existingOuterFunction, false);
+            Assert.assertSame(existingOuterFunction, 
db.getFunction(searchDesc(existingOuterFunction)));
+
+            Mockito.clearInvocations(spyEditLog);
+            Function tableFunction = createJavaUdtf(
+                    "rollback_table_function_conflict_db", 
"rollback_table_conflict_fn", Type.INT);
+            UserException exception = Assert.assertThrows(UserException.class,
+                    () -> db.addTableFunction(tableFunction, false));
+            Assert.assertEquals("function already exists", 
exception.getMessage());

Review Comment:
   This assertion will not match the `UserException` thrown by the conflict 
path. `FunctionUtil.addFunctionImpl()` throws `new UserException("function 
already exists")`, but `UserException.getMessage()` formats it as `errCode = 2, 
detailMessage = function already exists`; the bare text is returned by 
`getDetailMessage()`. As written, this new test fails on the assertion instead 
of validating rollback. Please assert `exception.getDetailMessage()` here, or 
compare `getMessage()` against the full formatted 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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to