This is an automated email from the ASF dual-hosted git repository.
yashmayya pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new c7ce654d81 Schema and table config deletion to validate with logical
table ref table names (#15900)
c7ce654d81 is described below
commit c7ce654d819582a641dcede5b3e177e3a038d9da
Author: Abhishek Bafna <[email protected]>
AuthorDate: Tue Jun 3 19:34:09 2025 +0530
Schema and table config deletion to validate with logical table ref table
names (#15900)
---
.../common/utils/LogicalTableConfigUtils.java | 41 +++++++++++
.../api/resources/PinotSchemaRestletResource.java | 7 ++
.../api/resources/PinotTableRestletResource.java | 17 +++++
.../api/resources/TableConfigsRestletResource.java | 20 ++++++
.../api/PinotSchemaRestletResourceTest.java | 40 +++++++++++
.../api/PinotTableRestletResourceTest.java | 80 ++++++++++++++++++++--
.../api/TableConfigsRestletResourceTest.java | 60 ++++++++++++++++
.../resources/PinotLogicalTableResourceTest.java | 62 ++++++++++++++---
.../pinot/controller/helix/ControllerTest.java | 9 +++
9 files changed, 324 insertions(+), 12 deletions(-)
diff --git
a/pinot-common/src/main/java/org/apache/pinot/common/utils/LogicalTableConfigUtils.java
b/pinot-common/src/main/java/org/apache/pinot/common/utils/LogicalTableConfigUtils.java
index 42d5e43f9f..1f9d948728 100644
---
a/pinot-common/src/main/java/org/apache/pinot/common/utils/LogicalTableConfigUtils.java
+++
b/pinot-common/src/main/java/org/apache/pinot/common/utils/LogicalTableConfigUtils.java
@@ -31,6 +31,7 @@ import org.apache.helix.zookeeper.datamodel.ZNRecord;
import org.apache.pinot.common.metadata.ZKMetadataProvider;
import org.apache.pinot.spi.config.table.QueryConfig;
import org.apache.pinot.spi.config.table.QuotaConfig;
+import org.apache.pinot.spi.config.table.TableType;
import org.apache.pinot.spi.data.LogicalTableConfig;
import org.apache.pinot.spi.data.PhysicalTableConfig;
import org.apache.pinot.spi.data.TimeBoundaryConfig;
@@ -164,12 +165,40 @@ public class LogicalTableConfigUtils {
}
}
+ // validate ref offline table name is offline table type
+ if (logicalTableConfig.getRefOfflineTableName() != null
+ &&
!TableNameBuilder.isOfflineTableResource(logicalTableConfig.getRefOfflineTableName()))
{
+ throw new IllegalArgumentException(
+ "Invalid logical table. Reason: 'refOfflineTableName' should be an
offline table type");
+ }
+
+ // validate ref realtime table name is realtime table type
+ if (logicalTableConfig.getRefRealtimeTableName() != null
+ &&
!TableNameBuilder.isRealtimeTableResource(logicalTableConfig.getRefRealtimeTableName()))
{
+ throw new IllegalArgumentException(
+ "Invalid logical table. Reason: 'refRealtimeTableName' should be a
realtime table type");
+ }
+
// validate ref offline table name is not null or empty when offline
tables exists
if (!offlineTableNames.isEmpty() &&
StringUtils.isEmpty(logicalTableConfig.getRefOfflineTableName())) {
throw new IllegalArgumentException(
"Invalid logical table. Reason: 'refOfflineTableName' should not be
null or empty when offline table exists");
}
+ // validate ref offline table name is null when offline tables is empty
+ if (offlineTableNames.isEmpty() &&
!StringUtils.isEmpty(logicalTableConfig.getRefOfflineTableName())) {
+ throw new IllegalArgumentException(
+ "Invalid logical table. Reason: 'refOfflineTableName' should be null
or empty when offline tables do not "
+ + "exist");
+ }
+
+ // validate ref realtime table name is null when realtime tables is empty
+ if (realtimeTableNames.isEmpty() &&
!StringUtils.isEmpty(logicalTableConfig.getRefRealtimeTableName())) {
+ throw new IllegalArgumentException(
+ "Invalid logical table. Reason: 'refRealtimeTableName' should be
null or empty when realtime tables do not "
+ + "exist");
+ }
+
// validate ref realtime table name is not null or empty when realtime
tables exists
if (!realtimeTableNames.isEmpty() &&
StringUtils.isEmpty(logicalTableConfig.getRefRealtimeTableName())) {
throw new IllegalArgumentException(
@@ -229,4 +258,16 @@ public class LogicalTableConfigUtils {
"Invalid logical table. Reason: 'timeBoundaryConfig.parameters'
should not be null or empty");
}
}
+
+ public static boolean checkPhysicalTableRefExists(LogicalTableConfig
logicalTableConfig, String tableName) {
+ Set<String> physicalTableNames =
logicalTableConfig.getPhysicalTableConfigMap().keySet();
+ TableType tableType =
TableNameBuilder.getTableTypeFromTableName(tableName);
+ if (tableType == null) {
+ String offlineTableName =
TableNameBuilder.OFFLINE.tableNameWithType(tableName);
+ String realtimeTableName =
TableNameBuilder.REALTIME.tableNameWithType(tableName);
+ return physicalTableNames.contains(offlineTableName) ||
physicalTableNames.contains(realtimeTableName);
+ } else {
+ return physicalTableNames.contains(tableName);
+ }
+ }
}
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
index 2e1d0d2c88..f2fcaa5093 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotSchemaRestletResource.java
@@ -57,6 +57,7 @@ import
org.apache.pinot.common.exception.SchemaAlreadyExistsException;
import org.apache.pinot.common.exception.SchemaBackwardIncompatibleException;
import org.apache.pinot.common.exception.SchemaNotFoundException;
import org.apache.pinot.common.exception.TableNotFoundException;
+import org.apache.pinot.common.metadata.ZKMetadataProvider;
import org.apache.pinot.common.metrics.ControllerMeter;
import org.apache.pinot.common.metrics.ControllerMetrics;
import org.apache.pinot.common.utils.DatabaseUtils;
@@ -551,6 +552,12 @@ public class PinotSchemaRestletResource {
Response.Status.CONFLICT);
}
}
+ // If the schema is associated with logical table, we should not delete it.
+ if
(ZKMetadataProvider.isLogicalTableExists(_pinotHelixResourceManager.getPropertyStore(),
schemaName)) {
+ throw new ControllerApplicationException(LOGGER,
+ String.format("Cannot delete schema %s, as it is associated with
logical table", schemaName),
+ Response.Status.CONFLICT);
+ }
LOGGER.info("Trying to delete schema {}", schemaName);
if (_pinotHelixResourceManager.deleteSchema(schemaName)) {
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
index 3302d9c7a5..30d8401165 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotTableRestletResource.java
@@ -83,6 +83,7 @@ import
org.apache.pinot.common.response.server.TableIndexMetadataResponse;
import org.apache.pinot.common.restlet.resources.TableSegmentValidationInfo;
import org.apache.pinot.common.restlet.resources.ValidDocIdsType;
import org.apache.pinot.common.utils.DatabaseUtils;
+import org.apache.pinot.common.utils.LogicalTableConfigUtils;
import org.apache.pinot.common.utils.helix.HelixHelper;
import org.apache.pinot.controller.ControllerConf;
import org.apache.pinot.controller.api.access.AccessControlFactory;
@@ -116,6 +117,7 @@ import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.config.table.TableStatsHumanReadable;
import org.apache.pinot.spi.config.table.TableStatus;
import org.apache.pinot.spi.config.table.TableType;
+import org.apache.pinot.spi.data.LogicalTableConfig;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.Enablement;
@@ -422,6 +424,7 @@ public class PinotTableRestletResource {
List<String> tablesDeleted = new LinkedList<>();
try {
tableName = DatabaseUtils.translateTableName(tableName, headers);
+ validateLogicalTableReference(tableName, tableType);
boolean tableExist = false;
if (verifyTableType(tableName, tableType, TableType.OFFLINE)) {
tableExist = _pinotHelixResourceManager.hasOfflineTable(tableName);
@@ -466,6 +469,20 @@ public class PinotTableRestletResource {
return typeFromTableName == null || typeFromTableName == expectedType;
}
+ private void validateLogicalTableReference(String tableName, TableType
tableType) {
+ String tableNameWithType =
+ tableType == null ? tableName :
TableNameBuilder.forType(tableType).tableNameWithType(tableName);
+ List<LogicalTableConfig> allLogicalTableConfigs =
+
ZKMetadataProvider.getAllLogicalTableConfigs(_pinotHelixResourceManager.getPropertyStore());
+ for (LogicalTableConfig logicalTableConfig : allLogicalTableConfigs) {
+ if
(LogicalTableConfigUtils.checkPhysicalTableRefExists(logicalTableConfig,
tableNameWithType)) {
+ throw new ControllerApplicationException(LOGGER,
+ String.format("Cannot delete table config: %s because it is
referenced in logical table: %s",
+ tableName, logicalTableConfig.getTableName()),
Response.Status.CONFLICT);
+ }
+ }
+ }
+
@PUT
@Path("/tables/{tableName}")
@Authorize(targetType = TargetType.TABLE, paramName = "tableName", action =
Actions.Table.UPDATE_TABLE_CONFIG)
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
index 3c3d6e5b3d..3a15ecb30e 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/TableConfigsRestletResource.java
@@ -49,9 +49,11 @@ import javax.ws.rs.core.HttpHeaders;
import javax.ws.rs.core.MediaType;
import javax.ws.rs.core.Response;
import org.apache.commons.lang3.tuple.Pair;
+import org.apache.pinot.common.metadata.ZKMetadataProvider;
import org.apache.pinot.common.metrics.ControllerMeter;
import org.apache.pinot.common.metrics.ControllerMetrics;
import org.apache.pinot.common.utils.DatabaseUtils;
+import org.apache.pinot.common.utils.LogicalTableConfigUtils;
import org.apache.pinot.controller.ControllerConf;
import org.apache.pinot.controller.api.access.AccessControl;
import org.apache.pinot.controller.api.access.AccessControlFactory;
@@ -73,6 +75,7 @@ import org.apache.pinot.segment.local.utils.SchemaUtils;
import org.apache.pinot.segment.local.utils.TableConfigUtils;
import org.apache.pinot.spi.config.TableConfigs;
import org.apache.pinot.spi.config.table.TableConfig;
+import org.apache.pinot.spi.data.LogicalTableConfig;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.utils.JsonUtils;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
@@ -282,7 +285,24 @@ public class TableConfigsRestletResource {
@ApiParam(value = "TableConfigs name i.e. raw table name", required =
true) @PathParam("tableName")
String tableName, @Context HttpHeaders headers) {
try {
+ if (TableNameBuilder.isOfflineTableResource(tableName) ||
TableNameBuilder.isRealtimeTableResource(tableName)) {
+ throw new ControllerApplicationException(LOGGER, "Invalid table name:
" + tableName + ". Use raw table name.",
+ Response.Status.BAD_REQUEST);
+ }
+
tableName = DatabaseUtils.translateTableName(tableName, headers);
+
+ // Validate the table is not referenced in any logical table config.
+ List<LogicalTableConfig> allLogicalTableConfigs =
+
ZKMetadataProvider.getAllLogicalTableConfigs(_pinotHelixResourceManager.getPropertyStore());
+ for (LogicalTableConfig logicalTableConfig : allLogicalTableConfigs) {
+ if
(LogicalTableConfigUtils.checkPhysicalTableRefExists(logicalTableConfig,
tableName)) {
+ throw new ControllerApplicationException(LOGGER,
+ String.format("Cannot delete table config: %s because it is
referenced in logical table: %s",
+ tableName, logicalTableConfig.getTableName()),
Response.Status.CONFLICT);
+ }
+ }
+
boolean tableExists =
_pinotHelixResourceManager.hasRealtimeTable(tableName) ||
_pinotHelixResourceManager.hasOfflineTable(
tableName);
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSchemaRestletResourceTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSchemaRestletResourceTest.java
index 15fa17c436..10af6b046b 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSchemaRestletResourceTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotSchemaRestletResourceTest.java
@@ -19,17 +19,23 @@
package org.apache.pinot.controller.api;
import java.io.IOException;
+import java.util.List;
import org.apache.pinot.common.utils.SimpleHttpResponse;
import org.apache.pinot.controller.helix.ControllerTest;
+import org.apache.pinot.spi.config.table.TableType;
import org.apache.pinot.spi.data.DimensionFieldSpec;
import org.apache.pinot.spi.data.FieldSpec.DataType;
+import org.apache.pinot.spi.data.LogicalTableConfig;
import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.utils.builder.ControllerRequestURLBuilder;
+import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.testng.annotations.AfterClass;
import org.testng.annotations.BeforeClass;
import org.testng.annotations.Test;
import static org.testng.Assert.assertEquals;
import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
import static org.testng.Assert.fail;
@@ -247,6 +253,40 @@ public class PinotSchemaRestletResourceTest {
"{\"unrecognizedProperties\":{\"/illegalKey1\":1},\"status\":\"transcript2
successfully added\"}");
}
+ @Test
+ public void testSchemaDeletionWithLogicalTable()
+ throws IOException {
+ String logicalTableName = "logical_table";
+ String physicalTable = "physical_table";
+ ControllerRequestURLBuilder urlBuilder =
TEST_INSTANCE.getControllerRequestURLBuilder();
+ TEST_INSTANCE.addDummySchema(physicalTable);
+
TEST_INSTANCE.addTableConfig(ControllerTest.createDummyTableConfig(physicalTable,
TableType.OFFLINE));
+ TEST_INSTANCE.addDummySchema(logicalTableName);
+
+ // Create a logical table
+ String logicalTableUrl = urlBuilder.forLogicalTableCreate();
+ LogicalTableConfig logicalTableConfig =
+ ControllerTest.getDummyLogicalTableConfig(logicalTableName,
+
List.of(TableNameBuilder.OFFLINE.tableNameWithType(physicalTable)),
"DefaultTenant");
+ String response = ControllerTest.sendPostRequest(logicalTableUrl,
logicalTableConfig.toSingleLineJsonString());
+ assertEquals(response,
+ "{\"unrecognizedProperties\":{},\"status\":\"logical_table logical
table successfully added.\"}");
+
+ // Delete schema should fail because logical table exists
+ String deleteSchemaUrl = urlBuilder.forSchemaDelete(logicalTableName);
+ String msg = expectThrows(IOException.class, () ->
ControllerTest.sendDeleteRequest(deleteSchemaUrl)).getMessage();
+ assertTrue(msg.contains("Cannot delete schema logical_table, as it is
associated with logical table"), msg);
+
+ // Delete logical table
+ String logicalTableDeleteUrl =
urlBuilder.forLogicalTableDelete(logicalTableName);
+ response = ControllerTest.sendDeleteRequest(logicalTableDeleteUrl);
+ assertEquals(response, "{\"status\":\"logical_table logical table
successfully deleted.\"}");
+
+ // Delete schema should succeed now
+ response = ControllerTest.sendDeleteRequest(deleteSchemaUrl);
+ assertEquals(response, "{\"status\":\"Schema logical_table deleted\"}");
+ }
+
@AfterClass
public void tearDown() {
TEST_INSTANCE.cleanup();
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java
index fc21084328..056228c38a 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/PinotTableRestletResourceTest.java
@@ -51,13 +51,21 @@ import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.stream.StreamConfig;
import org.apache.pinot.spi.utils.JsonUtils;
import org.apache.pinot.spi.utils.StringUtil;
+import org.apache.pinot.spi.utils.builder.ControllerRequestURLBuilder;
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.testng.annotations.AfterClass;
+import org.testng.annotations.AfterMethod;
import org.testng.annotations.BeforeClass;
+import org.testng.annotations.BeforeMethod;
import org.testng.annotations.Test;
-import static org.testng.Assert.*;
+import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
+import static org.testng.Assert.assertTrue;
+import static org.testng.Assert.expectThrows;
+import static org.testng.Assert.fail;
/**
@@ -79,15 +87,19 @@ public class PinotTableRestletResourceTest extends
ControllerTest {
_offlineBuilder.setTableName(OFFLINE_TABLE_NAME).setTimeColumnName("timeColumn").setTimeType("DAYS")
.setRetentionTimeUnit("DAYS").setRetentionTimeValue("5");
- // add schema for realtime table
- DEFAULT_INSTANCE.addDummySchema(REALTIME_TABLE_NAME);
- DEFAULT_INSTANCE.addDummySchema(OFFLINE_TABLE_NAME);
StreamConfig streamConfig =
FakeStreamConfigUtils.getDefaultLowLevelStreamConfigs();
_realtimeBuilder.setTableName(REALTIME_TABLE_NAME).setTimeColumnName("timeColumn").setTimeType("DAYS")
.setRetentionTimeUnit("DAYS").setRetentionTimeValue("5")
.setStreamConfigs(streamConfig.getStreamConfigsMap());
}
+ @BeforeMethod
+ public void beforeMethod()
+ throws Exception {
+ DEFAULT_INSTANCE.addDummySchema(REALTIME_TABLE_NAME);
+ DEFAULT_INSTANCE.addDummySchema(OFFLINE_TABLE_NAME);
+ }
+
private void registerMinionTasks() {
PinotTaskManager taskManager =
DEFAULT_INSTANCE.getControllerStarter().getTaskManager();
taskManager.registerTaskGenerator(new BaseTaskGenerator() {
@@ -634,6 +646,59 @@ public class PinotTableRestletResourceTest extends
ControllerTest {
assertEquals(deleteResponse, "{\"status\":\"Tables: [table3_OFFLINE]
deleted\"}");
}
+ @Test(dataProvider = "tableTypeProvider")
+ public void testDeleteTableWithLogicalTable(TableType tableType)
+ throws IOException {
+ ControllerRequestURLBuilder urlBuilder =
DEFAULT_INSTANCE.getControllerRequestURLBuilder();
+ String logicalTable = "logicalTable";
+ String tableName = "physicalTable";
+ String tableNameWithType =
TableNameBuilder.forType(tableType).tableNameWithType(tableName);
+ DEFAULT_INSTANCE.addDummySchema(tableName);
+ DEFAULT_INSTANCE.addDummySchema(logicalTable);
+ DEFAULT_INSTANCE.addTableConfig(createDummyTableConfig(tableName,
tableType));
+
+ LogicalTableConfig logicalTableConfig =
+ ControllerTest.getDummyLogicalTableConfig(logicalTable,
List.of(tableNameWithType), "DefaultTenant");
+ String logicalTableUrl = urlBuilder.forLogicalTableCreate();
+ String response = ControllerTest.sendPostRequest(logicalTableUrl,
logicalTableConfig.toSingleLineJsonString());
+ assertEquals(response,
+ "{\"unrecognizedProperties\":{},\"status\":\"logicalTable logical
table successfully added.\"}");
+
+ // table deletion should fail
+ String tableDeleteUrl = urlBuilder.forTableDelete(tableNameWithType);
+ String msg = expectThrows(IOException.class, () ->
ControllerTest.sendDeleteRequest(tableDeleteUrl)).getMessage();
+ assertTrue(msg.contains("Cannot delete table config: " + tableNameWithType
+ + " because it is referenced in logical table: logicalTable"), msg);
+
+ // table delete with name and type should also fail
+ msg = expectThrows(IOException.class,
+ () -> ControllerTest.sendDeleteRequest(tableDeleteUrl + "?type=" +
tableType)).getMessage();
+ assertTrue(msg.contains("Cannot delete table config: " + tableNameWithType
+ + " because it is referenced in logical table: logicalTable"), msg);
+
+ // table delete with raw table name also should fail
+ msg = expectThrows(IOException.class,
+ () ->
ControllerTest.sendDeleteRequest(urlBuilder.forTableDelete(tableName))).getMessage();
+ assertTrue(msg.contains(
+ "Cannot delete table config: " + tableName + " because it is
referenced in logical table: logicalTable"), msg);
+
+ // table delete with raw table name and type also should fail
+ msg = expectThrows(IOException.class,
+ () ->
ControllerTest.sendDeleteRequest(urlBuilder.forTableDelete(tableName + "?type="
+ tableType)))
+ .getMessage();
+ assertTrue(msg.contains(
+ "Cannot delete table config: " + tableName + " because it is
referenced in logical table: logicalTable"), msg);
+
+ // Delete logical table
+ String logicalTableDeleteUrl =
urlBuilder.forLogicalTableDelete(logicalTable);
+ response = ControllerTest.sendDeleteRequest(logicalTableDeleteUrl);
+ assertEquals(response, "{\"status\":\"logicalTable logical table
successfully deleted.\"}");
+
+ // Now table deletion should succeed
+ response = ControllerTest.sendDeleteRequest(tableDeleteUrl);
+ assertEquals(response, "{\"status\":\"Tables: [" + tableNameWithType + "]
deleted\"}");
+ }
+
@Test
public void testCheckTableState()
throws IOException {
@@ -998,6 +1063,13 @@ public class PinotTableRestletResourceTest extends
ControllerTest {
InstanceAssignmentConfig.PartitionSelector.FD_AWARE_INSTANCE_PARTITION_SELECTOR.name(),
false);
}
+ @AfterMethod
+ public void cleanUp()
+ throws IOException {
+ // Delete all tables after each test
+ DEFAULT_INSTANCE.cleanup();
+ }
+
@AfterClass
public void tearDown() {
DEFAULT_INSTANCE.stopSharedTestSetup();
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/TableConfigsRestletResourceTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/TableConfigsRestletResourceTest.java
index 54cc7b0ac0..7960f120dc 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/TableConfigsRestletResourceTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/TableConfigsRestletResourceTest.java
@@ -35,11 +35,13 @@ import org.apache.pinot.spi.config.table.TableType;
import org.apache.pinot.spi.config.table.TunerConfig;
import org.apache.pinot.spi.data.DateTimeFieldSpec;
import org.apache.pinot.spi.data.FieldSpec;
+import org.apache.pinot.spi.data.LogicalTableConfig;
import org.apache.pinot.spi.data.MetricFieldSpec;
import org.apache.pinot.spi.data.Schema;
import org.apache.pinot.spi.stream.StreamConfig;
import org.apache.pinot.spi.utils.CommonConstants;
import org.apache.pinot.spi.utils.JsonUtils;
+import org.apache.pinot.spi.utils.builder.ControllerRequestURLBuilder;
import org.apache.pinot.spi.utils.builder.TableConfigBuilder;
import org.apache.pinot.spi.utils.builder.TableNameBuilder;
import org.testng.Assert;
@@ -612,6 +614,64 @@ public class TableConfigsRestletResourceTest extends
ControllerTest {
Assert.assertEquals(configs.size(), 0);
}
+ @Test
+ public void testDeleteTableWithLogicalTable()
+ throws IOException {
+ String logicalTableName = "testDeleteLogicalTable";
+ String tableName = "physicalTable";
+ TableConfig offlineTableConfig = createOfflineTableConfig(tableName);
+ TableConfig realtimeTableConfig = createRealtimeTableConfig(tableName);
+ ControllerRequestURLBuilder urlBuilder =
DEFAULT_INSTANCE.getControllerRequestURLBuilder();
+ DEFAULT_INSTANCE.addDummySchema(logicalTableName);
+ DEFAULT_INSTANCE.addDummySchema(tableName);
+ DEFAULT_INSTANCE.addTableConfig(offlineTableConfig);
+ DEFAULT_INSTANCE.addTableConfig(realtimeTableConfig);
+
+ // Create logical table
+ String createLogicalTableUrl = urlBuilder.forLogicalTableCreate();
+ LogicalTableConfig logicalTableConfig =
getDummyLogicalTableConfig(logicalTableName,
+ List.of(offlineTableConfig.getTableName(),
realtimeTableConfig.getTableName()), "DefaultTenant");
+ String response = sendPostRequest(createLogicalTableUrl,
logicalTableConfig.toJsonString());
+ Assert.assertTrue(response.contains("testDeleteLogicalTable logical table
successfully added"), response);
+
+ // Delete table should fail because it is referenced by a logical table
+ String msg = Assert.expectThrows(
+ IOException.class, () ->
sendDeleteRequest(urlBuilder.forTableConfigsDelete(tableName))).getMessage();
+ Assert.assertTrue(msg.contains("Cannot delete table config: " + tableName
+ + " because it is referenced in logical table: " + logicalTableName),
msg);
+
+ // Delete logical table
+ String deleteLogicalTableUrl =
urlBuilder.forLogicalTableDelete(logicalTableName);
+ response = sendDeleteRequest(deleteLogicalTableUrl);
+ Assert.assertTrue(response.contains("testDeleteLogicalTable logical table
successfully deleted"), response);
+
+ // physical table should be deleted successfully
+ response = sendDeleteRequest(urlBuilder.forTableConfigsDelete(tableName));
+ Assert.assertTrue(response.contains("Deleted TableConfigs:
physicalTable"), response);
+ }
+
+ @Test
+ public void testDeleteTableConfigWithTableTypeValidation()
+ throws IOException {
+ ControllerRequestURLBuilder urlBuilder =
DEFAULT_INSTANCE.getControllerRequestURLBuilder();
+ String tableName = "testDeleteWithTypeValidation";
+ TableConfig offlineTableConfig = createOfflineTableConfig(tableName);
+ DEFAULT_INSTANCE.addDummySchema(tableName);
+ DEFAULT_INSTANCE.addTableConfig(offlineTableConfig);
+
+ // Delete table should fail because it is not a raw table name
+ String msg = Assert.expectThrows(
+ IOException.class,
+ () ->
sendDeleteRequest(urlBuilder.forTableConfigsDelete(offlineTableConfig.getTableName())))
+ .getMessage();
+ Assert.assertTrue(msg.contains("Invalid table name:
testDeleteWithTypeValidation_OFFLINE. Use raw table name."),
+ msg);
+
+ // Delete table with raw table name
+ String response =
sendDeleteRequest(urlBuilder.forTableConfigsDelete(tableName));
+ Assert.assertTrue(response.contains("Deleted TableConfigs:
testDeleteWithTypeValidation"), response);
+ }
+
@Test
public void testUnrecognizedProperties()
throws IOException {
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotLogicalTableResourceTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotLogicalTableResourceTest.java
index 50a29e2e55..47eb75d62c 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotLogicalTableResourceTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/api/resources/PinotLogicalTableResourceTest.java
@@ -217,6 +217,60 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
assertTrue(aThrows.getMessage()
.contains("Reason: 'refRealtimeTableName' should be one of the
provided realtime tables"),
aThrows.getMessage());
+
+ // Test ref offline table is not null but offline table does not exist
+ aThrows = expectThrows(
+ IOException.class, () -> {
+ LogicalTableConfig logicalTableConfig =
+ getDummyLogicalTableConfig(LOGICAL_TABLE_NAME,
List.of("test_table_7_REALTIME"), BROKER_TENANT);
+ logicalTableConfig.setRefOfflineTableName("test_table_7_OFFLINE");
+ ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(),
+ getHeaders());
+ }
+ );
+ assertTrue(aThrows.getMessage()
+ .contains("Reason: 'refOfflineTableName' should be null or empty
when offline tables do not exist"),
+ aThrows.getMessage());
+
+ // Test ref realtime table is not null but realtime table does not exist
+ aThrows = expectThrows(
+ IOException.class, () -> {
+ LogicalTableConfig logicalTableConfig =
+ getDummyLogicalTableConfig(LOGICAL_TABLE_NAME,
List.of("test_table_7_OFFLINE"), BROKER_TENANT);
+ logicalTableConfig.setRefRealtimeTableName("test_table_7_REALTIME");
+ ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(),
+ getHeaders());
+ }
+ );
+ assertTrue(aThrows.getMessage()
+ .contains("Reason: 'refRealtimeTableName' should be null or empty
when realtime tables do not exist"),
+ aThrows.getMessage());
+
+ // Test ref offline table name is realtime table name
+ aThrows = expectThrows(
+ IOException.class, () -> {
+ LogicalTableConfig logicalTableConfig =
+ getDummyLogicalTableConfig(LOGICAL_TABLE_NAME,
physicalTableNamesWithType, BROKER_TENANT);
+ logicalTableConfig.setRefOfflineTableName("test_table_7_REALTIME");
+ ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(),
+ getHeaders());
+ }
+ );
+ assertTrue(aThrows.getMessage().contains("Reason: 'refOfflineTableName'
should be an offline table type"),
+ aThrows.getMessage());
+
+ // Test ref realtime table name is offline table name
+ aThrows = expectThrows(
+ IOException.class, () -> {
+ LogicalTableConfig logicalTableConfig =
+ getDummyLogicalTableConfig(LOGICAL_TABLE_NAME,
physicalTableNamesWithType, BROKER_TENANT);
+ logicalTableConfig.setRefRealtimeTableName("test_table_7_OFFLINE");
+ ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(),
+ getHeaders());
+ }
+ );
+ assertTrue(aThrows.getMessage().contains("Reason: 'refRealtimeTableName'
should be a realtime table type"),
+ aThrows.getMessage());
}
@Test(expectedExceptions = IOException.class,
@@ -286,14 +340,6 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
throwable.getMessage());
}
- @DataProvider
- public Object[][] tableTypeProvider() {
- return new Object[][]{
- {TableType.OFFLINE},
- {TableType.REALTIME}
- };
- }
-
@Test(dataProvider = "tableTypeProvider")
public void testCreateLogicalTable(TableType tableType)
throws IOException {
diff --git
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
index cb04aed946..01092a3137 100644
---
a/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
+++
b/pinot-controller/src/test/java/org/apache/pinot/controller/helix/ControllerTest.java
@@ -96,6 +96,7 @@ import org.mockito.MockedStatic;
import org.mockito.Mockito;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import org.testng.annotations.DataProvider;
import static
org.apache.pinot.spi.utils.CommonConstants.Helix.UNTAGGED_BROKER_INSTANCE;
import static
org.apache.pinot.spi.utils.CommonConstants.Helix.UNTAGGED_SERVER_INSTANCE;
@@ -1247,6 +1248,14 @@ public class ControllerTest {
assertTrue(CollectionUtils.isEmpty(getHelixResourceManager().getSchemaNames()));
}
+ @DataProvider
+ public Object[][] tableTypeProvider() {
+ return new Object[][]{
+ {TableType.OFFLINE},
+ {TableType.REALTIME}
+ };
+ }
+
/**
* Clean shared state after a test case class has completed running.
Additional cleanup may be needed depending upon
* test functionality.
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
