swaminathanmanish commented on code in PR #14856:
URL: https://github.com/apache/pinot/pull/14856#discussion_r1925479045
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/mergerollup/MergeRollupTaskUtils.java:
##########
@@ -78,16 +79,59 @@ public static Map<String, Map<String, String>>
getLevelToConfigMap(Map<String, S
return levelToConfigMap;
}
+ /**
+ * Returns a lookup key composed of the current merge level / key combination
+ * @param key the key of the value within the task configuration.
+ * @param taskConfig the current merge rollup task configuration used for
sourcing the merge level.
+ * @return composite lookup key if the merge level is configured.
Otherwise, return original key.
+ */
+ public static String buildMergeLevelKeyPrefix(String key, Map<String,
String> taskConfig) {
Review Comment:
Wouldn't this have been an issue with existing merge rollup without your
changes as well ?
I guess your changes add a new functionality to perform custom
transformation based on merge level and thats where this bug is surfaced?
"Any lookups on the configuration during task execution have to include the
mergeLevel prefix in order to resolve lookups correctly."
--
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]
