siddharthteotia commented on code in PR #8989:
URL: https://github.com/apache/pinot/pull/8989#discussion_r919530707
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -881,6 +884,18 @@ private static void validateFieldConfigList(@Nullable
List<FieldConfig> fieldCon
}
}
+ private static void validateTableGroupConfig(TableConfig tableConfig) {
+ if (StringUtils.isBlank(tableConfig.getTableGroupName())) {
+ return;
+ }
+
Preconditions.checkState(tableConfig.getValidationConfig().getReplicaGroupStrategyConfig()
!= null, "Must provide"
Review Comment:
This check may not be needed.
`replicaGroupStrategyConfig` is the legacy way (which at some point has to
be deleted so let's try not to leak it more in the code) of specifying replica
groups and partitioned replica groups config.
Since adding tables to `TableGroup` is wired via `TableGroupConfig` and the
`instanceAssignmentConfig` within TableGroupConfig, it is sufficient and you
don't need to enforce that each table within the TableGroup should also have
their own `replicaGroupStrategyConfig`.
Also, the current check does not really ensure that table is indeed
partitioned. I think we should also ensure that each table in the `TableGroup`
is partitioned on the same column.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
