Jackie-Jiang commented on code in PR #15733:
URL: https://github.com/apache/pinot/pull/15733#discussion_r2082341043
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1637,6 +1637,12 @@ public Schema getSchemaForTableConfig(TableConfig
tableConfig) {
return ZKMetadataProvider.getTableSchema(_propertyStore, tableConfig);
}
+ public List<String> getAllSchemaNames() {
Review Comment:
I feel it is a bug on line 1670 where it should `return schemas` instead
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/LogicalTableUtils.java:
##########
@@ -73,8 +74,11 @@ public static ZNRecord toZNRecord(LogicalTableConfig
logicalTableConfig)
return record;
}
- public static void validateLogicalTableName(LogicalTableConfig
logicalTableConfig, List<String> allPhysicalTables,
- Set<String> allBrokerTenantNames) {
+ public static void validateLogicalTableName(
+ LogicalTableConfig logicalTableConfig,
+ Predicate<String> physicalTableExistsPredicate,
+ Predicate<String> brokerTenantExistsPredicate,
+ ZkHelixPropertyStore<ZNRecord> propertyStore) {
Review Comment:
Can you make this also a `Predicate` to hide implementation details?
##########
pinot-controller/src/main/java/org/apache/pinot/controller/helix/core/PinotHelixResourceManager.java:
##########
@@ -1825,6 +1832,17 @@ public void addLogicalTableConfig(LogicalTableConfig
logicalTableConfig)
String tableName = logicalTableConfig.getTableName();
LOGGER.info("Adding logical table {}: Start", tableName);
+ if (StringUtils.isEmpty(logicalTableConfig.getBrokerTenant())) {
+ logicalTableConfig.setBrokerTenant("DefaultTenant");
+ }
+
+ LogicalTableUtils.validateLogicalTableName(
+ logicalTableConfig,
+
PinotHelixPropertyStoreZnRecordProvider.forTable(_propertyStore)::exist,
+ getAllBrokerTenantNames()::contains,
+ _propertyStore
+ );
Review Comment:
Consider extracting this as a helper method
--
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]
