xiangfu0 commented on code in PR #18932:
URL: https://github.com/apache/pinot/pull/18932#discussion_r3549302247
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -725,7 +725,7 @@ public static void validateIngestionConfig(TableConfig
tableConfig, Schema schem
+ "' for column '" + columnName + "'");
}
try {
- expressionEvaluator =
FunctionEvaluatorFactory.getExpressionEvaluator(transformFunction);
+ expressionEvaluator =
FunctionEvaluatorFactory.getDeterministicExpressionEvaluator(transformFunction);
Review Comment:
This makes normal controller validation reject existing table configs that
already have now()/ago()/rand() in ingestion transforms. Keeping
ExpressionTransformer permissive lets already-running tables continue
ingesting, but PUT /tables and related update paths validate the full
TableConfig before saving, so operators can be blocked from unrelated config
edits after upgrade unless they bypass validation entirely. Please make this
rollout-safe, e.g. only reject new/changed transforms, gate the stricter
validation behind a compatibility flag, or add an explicit migration path with
a regression test for updating an existing table that still has one of these
transforms.
--
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]
