Jackie-Jiang commented on code in PR #18956:
URL: https://github.com/apache/pinot/pull/18956#discussion_r3556911095
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -443,325 +442,371 @@ private static boolean isValidPeerDownloadScheme(String
peerSegmentDownloadSchem
||
CommonConstants.HTTPS_PROTOCOL.equalsIgnoreCase(peerSegmentDownloadScheme);
}
- /**
- * Validates the following:
- * 1. validity of filter function
- * 2. checks for duplicate transform configs
- * 3. checks for null column name or transform function in transform config
- * 4. validity of transform function string
- * 5. checks for source fields used in destination columns
- * 6. ingestion type for dimension tables
- */
+ /// Validates the table's [IngestionConfig] together with the closely
related metrics-aggregation config, covering:
+ /// - Metrics aggregation (both the `aggregateMetrics` flag and ingestion
`aggregationConfigs`), delegated to
+ /// [#validateMetricsAggregation].
+ /// - Batch ingestion: each batch config map is well-formed, and a dimension
table has batch ingestion configured
+ /// with `REFRESH` segment ingestion type.
+ /// - Stream ingestion: streams are declared in only one place, at least one
stream is present, and pauseless
+ /// consumption has a valid `peerSegmentDownloadScheme`.
+ /// - Filter config: the filter function is valid and not a disabled Groovy
expression.
+ /// - Source field configs: no source field is duplicated within the same
complex-type phase.
+ /// - Enrichment configs: each config is valid.
+ /// - Transform configs: non-null column and function, no duplicate
destination, and each destination is a schema
+ /// column, an intermediate consumed by another transform, or an
aggregation source column; the function is valid
+ /// and does not reference its own destination.
+ /// - Complex-type config: no schema field collides with a
`prefixesToRename` prefix.
+ /// - Schema-conforming transformer config.
@VisibleForTesting
public static void validateIngestionConfig(TableConfig tableConfig, Schema
schema) {
+ // All metrics-aggregation validation lives here; it returns the columns
referenced as aggregation sources, which
+ // a transform config is allowed to target as its destination (see the
transform validation below).
+ Set<String> aggregationSourceColumns =
validateMetricsAggregation(tableConfig, schema);
+
IngestionConfig ingestionConfig = tableConfig.getIngestionConfig();
+ if (ingestionConfig == null) {
+ return;
+ }
Review Comment:
This is behavior-equivalent to `master`: the dim-table batch-config check
previously lived inside `if (ingestionConfig != null) { ... }`, so it was
equally skipped when `ingestionConfig` is null. The early-return doesn't change
that, so it isn't a regression. The latent gap (a dimension table with no
`ingestionConfig` at all) predates this change and is out of scope for this
refactor.
--
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]
