swaminathanmanish commented on code in PR #14373: URL: https://github.com/apache/pinot/pull/14373#discussion_r1871045842
########## pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskUtils.java: ########## @@ -45,12 +47,25 @@ private MergeRollupTaskUtils() { */ public static Map<String, Map<String, String>> getLevelToConfigMap(Map<String, String> taskConfig) { Map<String, Map<String, String>> levelToConfigMap = new TreeMap<>(); + + // Regex to match aggregation function parameter keys + Pattern pattern = Pattern.compile("(\\w+)\\.aggregationFunctionParameters\\.(\\w+)\\.(\\w+)"); + for (Map.Entry<String, String> entry : taskConfig.entrySet()) { String key = entry.getKey(); for (String configKey : VALID_CONFIG_KEYS) { if (key.endsWith(configKey)) { String level = key.substring(0, key.length() - configKey.length() - 1); levelToConfigMap.computeIfAbsent(level, k -> new TreeMap<>()).put(configKey, entry.getValue()); + } else { + Matcher matcher = pattern.matcher(key); Review Comment: Thanks for explanation. Looks like there are 2 types of configs (overall task level config to distribute work/sizing of segments etc) and there's app specific config (like nominal entries etc..). Would it be easy to extend your logic to add more keys? -- 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