aishikbh commented on code in PR #12032:
URL: https://github.com/apache/pinot/pull/12032#discussion_r1402046515


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/CompositeTransformer.java:
##########
@@ -68,13 +68,18 @@ public class CompositeTransformer implements 
RecordTransformer {
    *     Optional {@link SanitizationTransformer} after {@link 
NullValueTransformer} so that before sanitation, all
    *     values are non-null and follow the data types defined in the schema
    *   </li>
+   *   <li>
+   *     {@link SpecialValueTransformer} after {@link DataTypeTransformer} so 
that we already have the values complying
+   *      with the schema before handling special values
+   *   </li>
    * </ul>
    */
   public static List<RecordTransformer> getDefaultTransformers(TableConfig 
tableConfig, Schema schema) {
     return Stream.of(new ExpressionTransformer(tableConfig, schema), new 
FilterTransformer(tableConfig),
-        new SchemaConformingTransformer(tableConfig, schema), new 
DataTypeTransformer(tableConfig, schema),
-        new TimeValidationTransformer(tableConfig, schema), new 
NullValueTransformer(tableConfig, schema),
-        new SanitizationTransformer(schema)).filter(t -> 
!t.isNoOp()).collect(Collectors.toList());
+            new SchemaConformingTransformer(tableConfig, schema), new 
DataTypeTransformer(tableConfig, schema),

Review Comment:
   Makes sense. I have added a test to check and ensure the order of 
transformers.



-- 
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]

Reply via email to