diff --git a/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java b/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java index b5f79cc9dcc..f992854a44c 100644 --- a/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java +++ b/fe/fe-core/src/main/java/com/starrocks/sql/parser/AstBuilder.java @@ -4770,7 +4770,7 @@ public class AstBuilder extends com.starrocks.sql.parser.StarRocksBaseVisitor hostPortPairs = dropBackendClause.getHostPortPairs(); boolean needCheckWithoutForce = !dropBackendClause.isForce(); - String warehouse = dropBackendClause.getWarehouse(); - // check if the warehouse exist - if (GlobalStateMgr.getCurrentState().getWarehouseMgr().getWarehouseAllowNull(warehouse) == null) { - ErrorReport.reportDdlException(ErrorCode.ERR_UNKNOWN_WAREHOUSE, String.format("name: %s", warehouse)); - } - for (HostPort hostPort : hostPortPairs) { - dropBackend(hostPort.getHost(), hostPort.getPort(), warehouse, dropBackendClause.cngroupName, needCheckWithoutForce); + dropBackend(hostPort.getHost(), hostPort.getPort(), dropBackendClause.getWarehouse(), dropBackendClause.cngroupName, + needCheckWithoutForce); } } @@ -708,6 +703,11 @@ public class SystemInfoService implements GsonPostProcessable { } if (!Strings.isNullOrEmpty(warehouse)) { + // check if the warehouse exist + if (GlobalStateMgr.getCurrentState().getWarehouseMgr().getWarehouseAllowNull(warehouse) == null) { + ErrorReport.reportDdlException(ErrorCode.ERR_UNKNOWN_WAREHOUSE, String.format("name: %s", warehouse)); + } + // check if warehouseName is right Warehouse wh = GlobalStateMgr.getCurrentState().getWarehouseMgr() .getWarehouseAllowNull(droppedBackend.getWarehouseId()); diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/ast/DropBackendClauseTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/ast/DropBackendClauseTest.java index 6edada78c79..72e9199e42a 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/ast/DropBackendClauseTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/ast/DropBackendClauseTest.java @@ -32,6 +32,16 @@ public class DropBackendClauseTest { Assertions.assertTrue(dropStmt.getCNGroupName().isEmpty()); Assertions.assertFalse(dropStmt.isForce()); } + @Test + public void testDropBackendWithoutWarehouse() { + String sqlText = "ALTER SYSTEM DROP BACKEND 'backend01:9010' "; + AlterSystemStmt stmt = + (AlterSystemStmt) SqlParser.parseSingleStatement(sqlText, SqlModeHelper.MODE_DEFAULT); + DropBackendClause dropStmt = (DropBackendClause) stmt.getAlterClause(); + Assertions.assertTrue(dropStmt.getWarehouse().isEmpty()); + Assertions.assertTrue(dropStmt.getCNGroupName().isEmpty()); + Assertions.assertFalse(dropStmt.isForce()); + } @Test public void testDropBackendIntoWarehouseCnGroup() { diff --git a/fe/fe-core/src/test/java/com/starrocks/sql/ast/DropComputeNodeClauseTest.java b/fe/fe-core/src/test/java/com/starrocks/sql/ast/DropComputeNodeClauseTest.java index 42e6f29e771..04ae9e8f05e 100644 --- a/fe/fe-core/src/test/java/com/starrocks/sql/ast/DropComputeNodeClauseTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/sql/ast/DropComputeNodeClauseTest.java @@ -31,6 +31,15 @@ public class DropComputeNodeClauseTest { Assertions.assertEquals("warehouse1", dropStmt.getWarehouse()); Assertions.assertTrue(dropStmt.getCNGroupName().isEmpty()); } + @Test + public void testDropComputeNodeWithoutWarehouse() { + String sqlText = "ALTER SYSTEM DROP COMPUTE NODE 'backend01:9010'"; + AlterSystemStmt stmt = + (AlterSystemStmt) SqlParser.parseSingleStatement(sqlText, SqlModeHelper.MODE_DEFAULT); + DropComputeNodeClause dropStmt = (DropComputeNodeClause) stmt.getAlterClause(); + Assertions.assertTrue(dropStmt.getWarehouse().isEmpty()); + Assertions.assertTrue(dropStmt.getCNGroupName().isEmpty()); + } @Test public void testDropComputeNodeIntoWarehouseCnGroup() { diff --git a/fe/fe-core/src/test/java/com/starrocks/system/SystemInfoServiceTest.java b/fe/fe-core/src/test/java/com/starrocks/system/SystemInfoServiceTest.java index 188edada831..fa838ce59a0 100644 --- a/fe/fe-core/src/test/java/com/starrocks/system/SystemInfoServiceTest.java +++ b/fe/fe-core/src/test/java/com/starrocks/system/SystemInfoServiceTest.java @@ -17,6 +17,7 @@ package com.starrocks.system; import com.google.api.client.util.Maps; import com.starrocks.common.Config; import com.starrocks.common.DdlException; +import com.starrocks.common.ErrorCode; import com.starrocks.common.Pair; import com.starrocks.lake.StarOSAgent; import com.starrocks.persist.DropBackendInfo; @@ -288,6 +289,52 @@ public class SystemInfoServiceTest { ComputeNode cnIP = service.getComputeNodeWithHeartbeatPort("newHost", 1000); Assertions.assertTrue(cnIP == null); + Config.enable_trace_historical_node = savedConfig; + } + @Test + public void testDropComputeNode2() throws Exception { + new MockUp() { + @Mock + public RunMode getCurrentRunMode() { + return RunMode.SHARED_DATA; + } + }; + + Boolean savedConfig = Config.enable_trace_historical_node; + Config.enable_trace_historical_node = true; + + ComputeNode cn = new ComputeNode(10001, "newHost", 1000); + service.addComputeNode(cn); + + WarehouseManager warehouseManager = new WarehouseManager(); + warehouseManager.initDefaultWarehouse(); + + new MockUp() { + @Mock + public ComputeResource acquireComputeResource(CRAcquireContext acquireContext) { + return WarehouseManager.DEFAULT_RESOURCE; + } + }; + + new MockUp() { + @Mock + public void logDropComputeNode(DropComputeNodeLog log, WALApplier applier) { + applier.apply(log); + } + }; + cn.setStarletPort(1001); + + { + Assertions.assertThrows(DdlException.class, () -> + service.dropComputeNode("newHost", 1000, "warehousename-cn-not-exists", ""), + ErrorCode.ERR_UNKNOWN_WAREHOUSE.formatErrorMsg("name: warehousename-cn-not-exists")); + } + + service.dropComputeNode("newHost", 1000, "", ""); + ComputeNode cnIP = service.getComputeNodeWithHeartbeatPort("newHost", 1000); + Assertions.assertNull(cnIP); + + Config.enable_trace_historical_node = savedConfig; } @@ -337,6 +384,14 @@ public class SystemInfoServiceTest { }; service.addBackend(be); be.setStarletPort(1001); + + { + Assertions.assertThrows(DdlException.class, () -> + service.dropBackend("newHost", 1000, "warehousename-cn-not-exists", + "", false), + ErrorCode.ERR_UNKNOWN_WAREHOUSE.formatErrorMsg("name: warehousename-cn-not-exists")); + } + service.dropBackend("newHost", 1000, null, null, false); Backend beIP = service.getBackendWithHeartbeatPort("newHost", 1000); Assertions.assertNull(beIP);