capistrant commented on code in PR #19597:
URL: https://github.com/apache/druid/pull/19597#discussion_r3431700191
##########
server/src/main/java/org/apache/druid/client/indexing/ClientCompactionRunnerInfo.java:
##########
@@ -99,6 +99,11 @@ public static CompactionConfigValidationResult
validateCompactionConfig(
{
CompactionEngine compactionEngine = newConfig.getEngine() == null ?
defaultCompactionEngine : newConfig.getEngine();
if (compactionEngine == CompactionEngine.NATIVE) {
+ if (newConfig.getBaseTable() != null) {
+ return CompactionConfigValidationResult.failure(
+ "Compaction engine[native] does not support 'baseTable'; use the
MSQ compaction engine."
+ );
+ }
return CompactionConfigValidationResult.success();
} else {
return compactionConfigSupportedByMSQEngine(newConfig);
Review Comment:
what about adding a `baseTable != null` block to this MSQ validation method
that blows up if the config is trying to set the top level data schema configs
that are now handled by the base table. fuzzy to me on if that situation would
be handled okay (base table wins or blow up later on), but blowing up eagerly
feels safer and will prompt operator to consciously modify the config (or
rules?) to make it play nice
--
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]
