Copilot commented on code in PR #18956:
URL: https://github.com/apache/pinot/pull/18956#discussion_r3555670508
##########
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:
validateIngestionConfig() returns early when ingestionConfig is null, which
makes the dimension-table requirement check below ("Dimension tables must have
batch ingestion configuration") unreachable. As a result, an OFFLINE dimension
table can pass validation without any batch ingestion config, contrary to the
explicit Preconditions check later in this method.
--
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]
