xiangfu0 commented on code in PR #19244:
URL: https://github.com/apache/pinot/pull/19244#discussion_r3773822939
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -620,7 +621,9 @@ static void validateIngestionConfig(TableConfig
tableConfig, Schema schema,
+ columnName + "'");
}
try {
- validateIngestionTransformFunctionVolatility(transformConfig,
existingTransformConfigs);
+ if (tableConfig.getTableType() == TableType.REALTIME) {
+ validateIngestionTransformFunctionVolatility(transformConfig,
existingTransformConfigs);
+ }
Review Comment:
Agreed—this is a real merge blocker, not only a theoretical risk. I
reproduced it on the current branch with a 20-row OFFLINE segment using `r =
rand()`: validation and segment creation succeeded, but readback returned 20
rows with only one distinct value because second-pass values missing from the
first-pass dictionary resolved to dictionary ID 0.
I have converted the PR back to draft. An opt-in flag alone would still
permit silently incorrect segments, so I am leaving this thread unresolved
until row-based segment generation evaluates each transform once per input row
per build attempt and reuses/materializes that exact row for both statistics
and indexing. The fix also needs end-to-end build/readback coverage for
dictionary and no-dictionary outputs, asserting stored values, cardinality, and
min/max metadata.
--
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]
