xiangfu0 commented on code in PR #18724:
URL: https://github.com/apache/pinot/pull/18724#discussion_r3391322207
##########
pinot-plugins/pinot-minion-tasks/pinot-minion-builtin-tasks/src/main/java/org/apache/pinot/plugin/minion/tasks/realtimetoofflinesegments/RealtimeToOfflineSegmentsTaskGenerator.java:
##########
@@ -337,20 +339,27 @@ public void validateTaskConfigs(TableConfig tableConfig,
Schema schema, Map<Stri
Set<String> columnNames = schema.getColumnNames();
for (Map.Entry<String, String> entry : taskConfigs.entrySet()) {
if (entry.getKey().endsWith(".aggregationType")) {
- Preconditions.checkState(columnNames.contains(
- StringUtils.removeEnd(entry.getKey(),
RealtimeToOfflineSegmentsTask.AGGREGATION_TYPE_KEY_SUFFIX)),
- String.format("Column \"%s\" not found in schema!",
entry.getKey()));
+ String column =
+ StringUtils.removeEnd(entry.getKey(),
RealtimeToOfflineSegmentsTask.AGGREGATION_TYPE_KEY_SUFFIX);
+ Preconditions.checkState(columnNames.contains(column),
+ String.format("Column \"%s\" not found in schema!", column));
+ AggregationFunctionType aft;
try {
// check that it's a valid aggregation function type
- AggregationFunctionType aft =
AggregationFunctionType.getAggregationFunctionType(entry.getValue());
+ aft = MergeTaskUtils.getAggregationFunctionType(entry.getValue());
// check that a value aggregator is available
if
(!MinionConstants.RealtimeToOfflineSegmentsTask.AVAILABLE_CORE_VALUE_AGGREGATORS.contains(aft))
{
throw new IllegalArgumentException("ValueAggregator not enabled
for type: " + aft.toString());
}
} catch (IllegalArgumentException e) {
- String err =
- String.format("Column \"%s\" has invalid aggregate type: %s",
entry.getKey(), entry.getValue());
- throw new IllegalStateException(err);
+ String err = String.format("Column \"%s\" has invalid aggregate
type: %s", column, entry.getValue());
+ throw new IllegalStateException(err, e);
+ }
+ if (aft == AggregationFunctionType.FIRSTWITHTIME || aft ==
AggregationFunctionType.LASTWITHTIME) {
Review Comment:
Fixed in b7ed8c1 — RTO validation now rejects first/last unless the table
has a time column that resolves to a DateTime column in schema, and the
MergeRollup guard was strengthened the same way (it previously only checked the
time column was configured, not resolvable). Added negative test cases for both
generators.
--
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]
