xiangfu0 commented on code in PR #18932:
URL: https://github.com/apache/pinot/pull/18932#discussion_r3549702205
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/recordtransformer/ExpressionTransformerTest.java:
##########
@@ -245,6 +245,31 @@ public void
testTransformReturningNullDoesNotOverrideExistingBytesValue() {
Assert.assertFalse(row.isNullValue("payload"));
}
+ @Test
+ public void testNonDeterministicTransformFunctionStillRunsAtRuntime() {
+ Schema schema = new Schema.SchemaBuilder()
+ .addSingleValueDimension("eventTimeMs", FieldSpec.DataType.LONG)
+ .build();
Review Comment:
Clarified the scope in code by renaming this to
`testLegacyNonDeterministicTransformFunctionRemainsRuntimeCompatible`. The PR
intentionally keeps `ExpressionTransformer` permissive for existing table
configs and enforces the new rule in table validation for new/changed
transforms only.
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/recordtransformer/ExpressionTransformerTest.java:
##########
@@ -245,6 +245,31 @@ public void
testTransformReturningNullDoesNotOverrideExistingBytesValue() {
Assert.assertFalse(row.isNullValue("payload"));
}
+ @Test
+ public void testNonDeterministicTransformFunctionStillRunsAtRuntime() {
+ Schema schema = new Schema.SchemaBuilder()
+ .addSingleValueDimension("eventTimeMs", FieldSpec.DataType.LONG)
+ .build();
+ IngestionConfig ingestionConfig = new IngestionConfig();
+ ingestionConfig.setTransformConfigs(List.of(new
TransformConfig("eventTimeMs", "now()")));
+ TableConfig tableConfig = new TableConfigBuilder(TableType.REALTIME)
+
.setTableName("testNonDeterministicTransformFunctionStillRunsAtRuntime")
+ .setIngestionConfig(ingestionConfig)
+ .build();
+ ExpressionTransformer expressionTransformer = new
ExpressionTransformer(tableConfig, schema);
+
+ GenericRow row = new GenericRow();
+ long lowerBound = System.currentTimeMillis();
+ expressionTransformer.transform(row);
+ long upperBound = System.currentTimeMillis();
+
+ Object value = row.getValue("eventTimeMs");
+ Assert.assertTrue(value instanceof Long, "Expected now() transform to
produce a LONG value");
+ long eventTimeMs = (Long) value;
+ Assert.assertTrue(eventTimeMs >= lowerBound);
+ Assert.assertTrue(eventTimeMs <= upperBound);
Review Comment:
Added a 1s tolerance around the wall-clock bounds to reduce CI flakiness
while still checking that `now()` produced a current timestamp.
--
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]
