Jackie-Jiang commented on code in PR #12032:
URL: https://github.com/apache/pinot/pull/12032#discussion_r1399821015
##########
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:
We should add test to ensure the expected behavior. There is java doc above
the method: `NOTE: DO NOT CHANGE THE ORDER OF THE RECORD 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]
