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 a1e9f0a0554 Database name validation for logical tables (#15994)
a1e9f0a0554 is described below
commit a1e9f0a05545d768bbe8d6aac63dec4b0d4b89ea
Author: Abhishek Bafna <[email protected]>
AuthorDate: Tue Jun 17 09:16:54 2025 +0530
Database name validation for logical tables (#15994)
---
.../common/utils/LogicalTableConfigUtils.java | 10 +++
.../api/resources/PinotLogicalTableResource.java | 25 +++++-
.../resources/PinotLogicalTableResourceTest.java | 100 ++++++++++++++++-----
3 files changed, 110 insertions(+), 25 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 1f9d948728b..e1ac8e352e1 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
@@ -139,6 +139,7 @@ public class LogicalTableConfigUtils {
"Invalid logical table. Reason: 'physicalTableConfigMap' should not
be null or empty");
}
+ String databaseName =
DatabaseUtils.extractDatabaseFromFullyQualifiedTableName(logicalTableConfig.getTableName());
Set<String> offlineTableNames = new HashSet<>();
Set<String> realtimeTableNames = new HashSet<>();
@@ -146,6 +147,15 @@ public class LogicalTableConfigUtils {
String physicalTableName = entry.getKey();
PhysicalTableConfig physicalTableConfig = entry.getValue();
+ // validate database name matches
+ String physicalTableDatabaseName =
DatabaseUtils.extractDatabaseFromFullyQualifiedTableName(physicalTableName);
+ if (!StringUtils.equalsIgnoreCase(databaseName,
physicalTableDatabaseName)) {
+ throw new IllegalArgumentException(
+ "Invalid logical table. Reason: '" + physicalTableName
+ + "' should have the same database name as logical table: " +
databaseName + " != "
+ + physicalTableDatabaseName);
+ }
+
// validate physical table exists
if (!physicalTableExistsPredicate.test(physicalTableName)) {
throw new IllegalArgumentException(
diff --git
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotLogicalTableResource.java
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotLogicalTableResource.java
index 56a9f02edd9..1192c8444fe 100644
---
a/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotLogicalTableResource.java
+++
b/pinot-controller/src/main/java/org/apache/pinot/controller/api/resources/PinotLogicalTableResource.java
@@ -29,6 +29,7 @@ import io.swagger.annotations.SecurityDefinition;
import io.swagger.annotations.SwaggerDefinition;
import java.util.List;
import java.util.Map;
+import java.util.stream.Collectors;
import javax.inject.Inject;
import javax.ws.rs.Consumes;
import javax.ws.rs.DELETE;
@@ -57,6 +58,7 @@ import org.apache.pinot.core.auth.Authorize;
import org.apache.pinot.core.auth.ManualAuthorization;
import org.apache.pinot.core.auth.TargetType;
import org.apache.pinot.spi.data.LogicalTableConfig;
+import org.apache.pinot.spi.data.PhysicalTableConfig;
import org.apache.pinot.spi.utils.JsonUtils;
import org.glassfish.grizzly.http.server.Request;
import org.slf4j.Logger;
@@ -141,10 +143,31 @@ public class PinotLogicalTableResource {
ResourceUtils.checkPermissionAndAccess(tableName, request, httpHeaders,
AccessType.CREATE,
Actions.Table.CREATE_TABLE, _accessControlFactory, LOGGER);
+ translatePhysicalTableNamesWithDB(logicalTableConfig, httpHeaders);
SuccessResponse successResponse = addLogicalTable(logicalTableConfig);
return new ConfigSuccessResponse(successResponse.getStatus(),
logicalTableConfigAndUnrecognizedProps.getRight());
}
+ private void translatePhysicalTableNamesWithDB(LogicalTableConfig
logicalTableConfig, HttpHeaders headers) {
+ // Translate physical table names to include the database name
+ Map<String, PhysicalTableConfig> physicalTableConfigMap =
logicalTableConfig.getPhysicalTableConfigMap().entrySet()
+ .stream()
+ .map(entry ->
Map.entry(DatabaseUtils.translateTableName(entry.getKey(), headers),
entry.getValue()))
+ .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue));
+
+ logicalTableConfig.setPhysicalTableConfigMap(physicalTableConfigMap);
+
+ // Translate refOfflineTableName and refRealtimeTableName to include the
database name
+ String refOfflineTableName = logicalTableConfig.getRefOfflineTableName();
+ if (refOfflineTableName != null) {
+
logicalTableConfig.setRefOfflineTableName(DatabaseUtils.translateTableName(refOfflineTableName,
headers));
+ }
+ String refRealtimeTableName = logicalTableConfig.getRefRealtimeTableName();
+ if (refRealtimeTableName != null) {
+
logicalTableConfig.setRefRealtimeTableName(DatabaseUtils.translateTableName(refRealtimeTableName,
headers));
+ }
+ }
+
@PUT
@Produces(MediaType.APPLICATION_JSON)
@Consumes(MediaType.APPLICATION_JSON)
@@ -170,7 +193,7 @@ public class PinotLogicalTableResource {
tableName = DatabaseUtils.translateTableName(tableName, headers);
logicalTableConfig.setTableName(tableName);
-
+ translatePhysicalTableNamesWithDB(logicalTableConfig, headers);
SuccessResponse successResponse = updateLogicalTable(logicalTableConfig);
return new ConfigSuccessResponse(successResponse.getStatus(),
logicalTableConfigAndUnrecognizedProps.getRight());
}
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 47eb75d62ca..f65c4a98b34 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
@@ -25,6 +25,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import org.apache.helix.model.IdealState;
+import org.apache.pinot.common.utils.DatabaseUtils;
import org.apache.pinot.common.utils.helix.HelixHelper;
import org.apache.pinot.controller.helix.ControllerTest;
import org.apache.pinot.controller.helix.core.PinotHelixResourceManager;
@@ -85,23 +86,23 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
@DataProvider
public Object[][] tableNamesProvider() {
return new Object[][]{
- {"test_logical_table", List.of("test_table_1", "test_table_2"),
List.of("test_table_3")},
- {"test_logical_table", List.of("test_table_1", "db.test_table_2"),
List.of("test_table_3")},
- {"test_logical_table", List.of("test_table_1", "test_table_2"),
List.of("db.test_table_3")},
- {"test_logical_table", List.of("db.test_table_1", "db.test_table_2"),
List.of("db.test_table_3")},
- {"test_table", List.of("db1.test_table", "db2.test_table"),
List.of("db3.test_table")},
- {"db0.test_table", List.of("db1.test_table", "db2.test_table"),
List.of("db3.test_table")},
- {"db.test_logical_table", List.of("test_table_1", "test_table_2"),
List.of("test_table_3")},
- {"db.test_logical_table", List.of("test_table_1", "db.test_table_2"),
List.of("test_table_3")},
- {"db.test_logical_table", List.of("test_table_1", "test_table_2"),
List.of("db.test_table_3")},
- {"db.test_logical_table", List.of("db.test_table_1",
"db.test_table_2"), List.of("db.test_table_3")},
+ {"test_logical_table", List.of("test_table_1", "test_table_2"),
List.of("test_table_3"), Map.of()},
+ {"db.test_logical_table", List.of("db.test_table_1",
"db.test_table_2"), List.of("db.test_table_3"), Map.of()},
+ {
+ "test_logical_table", List.of("db1.test_table_1",
"db1.test_table_2"), List.of("db1.test_table_3"), Map.of(
+ CommonConstants.DATABASE, "db1")
+ },
};
}
@Test(dataProvider = "tableNamesProvider")
public void testCreateUpdateDeleteLogicalTables(String logicalTableName,
List<String> physicalTableNames,
- List<String> physicalTablesToUpdate)
+ List<String> physicalTablesToUpdate, Map<String, String> dbHeaders)
throws IOException {
+ Map<String, String> headers = new HashMap<>(getHeaders());
+ headers.putAll(dbHeaders);
+ logicalTableName = DatabaseUtils.translateTableName(logicalTableName,
headers.get(CommonConstants.DATABASE));
+
addDummySchema(logicalTableName);
// verify logical table does not exist
String getLogicalTableUrl =
_controllerRequestURLBuilder.forLogicalTableGet(logicalTableName);
@@ -118,7 +119,7 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
// create logical table
String resp =
- ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(), getHeaders());
+ ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(), headers);
assertEquals(resp,
"{\"unrecognizedProperties\":{},\"status\":\"" + logicalTableName + "
logical table successfully added.\"}");
@@ -131,7 +132,7 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
logicalTableConfig = getDummyLogicalTableConfig(logicalTableName,
tableNameToUpdateWithType, BROKER_TENANT);
String response =
- ControllerTest.sendPutRequest(updateLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(), getHeaders());
+ ControllerTest.sendPutRequest(updateLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(), headers);
assertEquals(response,
"{\"unrecognizedProperties\":{},\"status\":\"" + logicalTableName + "
logical table successfully updated.\"}");
@@ -139,7 +140,7 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
verifyLogicalTableExists(getLogicalTableUrl, logicalTableConfig);
// delete logical table
- String deleteResponse =
ControllerTest.sendDeleteRequest(deleteLogicalTableUrl, getHeaders());
+ String deleteResponse =
ControllerTest.sendDeleteRequest(deleteLogicalTableUrl, headers);
assertEquals(deleteResponse, "{\"status\":\"" + logicalTableName + "
logical table successfully deleted.\"}");
// verify logical table is deleted
@@ -271,6 +272,55 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
);
assertTrue(aThrows.getMessage().contains("Reason: 'refRealtimeTableName'
should be a realtime table type"),
aThrows.getMessage());
+
+ // Test ref offline table is specified with a database prefix
+ aThrows = expectThrows(
+ IOException.class, () -> {
+ LogicalTableConfig logicalTableConfig =
+ getDummyLogicalTableConfig(LOGICAL_TABLE_NAME,
physicalTableNamesWithType, BROKER_TENANT);
+ logicalTableConfig.setRefOfflineTableName("db.test_table_7_OFFLINE");
+ ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(),
+ getHeaders());
+ }
+ );
+ assertTrue(
+ aThrows.getMessage().contains("Reason: 'refOfflineTableName' should be
one of the provided offline tables"),
+ aThrows.getMessage());
+
+ // Test ref realtime table is specified with a database prefix
+ aThrows = expectThrows(
+ IOException.class, () -> {
+ LogicalTableConfig logicalTableConfig =
+ getDummyLogicalTableConfig(LOGICAL_TABLE_NAME,
physicalTableNamesWithType, BROKER_TENANT);
+
logicalTableConfig.setRefRealtimeTableName("db.test_table_7_REALTIME");
+ ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(),
+ getHeaders());
+ }
+ );
+ assertTrue(
+ aThrows.getMessage().contains("Reason: 'refRealtimeTableName' should
be one of the provided realtime tables"),
+ aThrows.getMessage());
+ }
+
+ @Test
+ public void testLogicalTableDatabaseValidation() {
+ String logicalTableName = "db1.test_logical_table";
+ LogicalTableConfig logicalTableConfig =
+ getDummyLogicalTableConfig(logicalTableName,
List.of("test_table_1_OFFLINE", "test_table_2_REALTIME"),
+ BROKER_TENANT);
+ // Test add logical table with different database prefix
+ String msg = expectThrows(IOException.class, () -> {
+ addLogicalTableConfig(logicalTableConfig);
+ }).getMessage();
+ assertTrue(msg.contains(
+ "Reason: 'test_table_1_OFFLINE' should have the same database name as
logical table: db1 != default"), msg);
+
+ // Test update logical table with different database prefix
+ msg = expectThrows(IOException.class, () ->
updateLogicalTableConfig(logicalTableConfig)).getMessage();
+ assertTrue(
+ msg.contains(
+ "Reason: 'test_table_1_OFFLINE' should have the same database name
as logical table: db1 != default"),
+ msg);
}
@Test(expectedExceptions = IOException.class,
@@ -374,7 +424,8 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
throwable = expectThrows(IOException.class, () -> {
addDummySchema(LOGICAL_TABLE_NAME);
LogicalTableConfig logicalTableConfig =
- getDummyLogicalTableConfig("db." + LOGICAL_TABLE_NAME,
physicalTableNamesWithType, BROKER_TENANT);
+ getDummyLogicalTableConfig("db." + LOGICAL_TABLE_NAME,
createHybridTables(List.of("db.test_table_6")),
+ BROKER_TENANT);
ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(), getHeaders());
});
assertTrue(throwable.getMessage()
@@ -463,8 +514,8 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
return new Object[][]{
{LOGICAL_TABLE_NAME, List.of("test_table_1"), "unknown_table_OFFLINE"},
{LOGICAL_TABLE_NAME, List.of("test_table_2"),
"unknown_table_REALTIME"},
- {LOGICAL_TABLE_NAME, List.of("test_table_1"),
"db.test_table_1_OFFLINE"},
- {LOGICAL_TABLE_NAME, List.of("test_table_2"),
"db.test_table_2_REALTIME"},
+ {"db." + LOGICAL_TABLE_NAME, List.of("db.test_table_1"),
"db.unknown_table_OFFLINE"},
+ {"db." + LOGICAL_TABLE_NAME, List.of("db.test_table_2"),
"db.unknown_table_REALTIME"},
};
}
@@ -472,6 +523,7 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
public void testPhysicalTableShouldExist(String logicalTableName,
List<String> physicalTableNames,
String unknownTableName)
throws IOException {
+ addDummySchema(logicalTableName);
// setup physical tables
List<String> physicalTableNamesWithType =
createHybridTables(physicalTableNames);
physicalTableNamesWithType.add(unknownTableName);
@@ -499,13 +551,14 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
// setup physical tables and logical tables
List<String> logicalTableNames =
List.of("db.test_logical_table_1", "default.test_logical_table_2",
"test_logical_table_3");
- List<String> physicalTableNames = List.of("test_table_1", "test_table_2",
"db.test_table_3");
- List<String> physicalTableNamesWithType =
createHybridTables(physicalTableNames);
for (int i = 0; i < logicalTableNames.size(); i++) {
- addDummySchema(logicalTableNames.get(i));
- LogicalTableConfig logicalTableConfig =
getDummyLogicalTableConfig(logicalTableNames.get(i), List.of(
- physicalTableNamesWithType.get(2 * i),
physicalTableNamesWithType.get(2 * i + 1)), BROKER_TENANT);
+ String logicalTableName = logicalTableNames.get(i);
+ String databaseName =
DatabaseUtils.extractDatabaseFromFullyQualifiedTableName(logicalTableName);
+ List<String> physicalTableNamesWithType =
createHybridTables(List.of(databaseName + ".test_table_" + i));
+ addDummySchema(logicalTableName);
+ LogicalTableConfig logicalTableConfig =
+ getDummyLogicalTableConfig(logicalTableName,
physicalTableNamesWithType, BROKER_TENANT);
ControllerTest.sendPostRequest(_addLogicalTableUrl,
logicalTableConfig.toSingleLineJsonString(), getHeaders());
}
@@ -524,8 +577,7 @@ public class PinotLogicalTableResourceTest extends
ControllerTest {
}
@Test
- public void testLogicalTableDatabaseHeaderMismatchValidation()
- throws IOException {
+ public void testLogicalTableDatabaseHeaderMismatchValidation() {
Map<String, String> headers = new HashMap<>(getHeaders());
headers.put(CommonConstants.DATABASE, "db1");
String logicalTableName = "db2.test_logical_table";
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
