[GitHub] [incubator-pinot] amrishlal commented on a change in pull request #6845: Add the complex data type transformer for complex type handling

[GitBox]

amrishlal commented on a change in pull request #6845:
URL: https://github.com/apache/incubator-pinot/pull/6845#discussion_r623273222



##########
File path: 
pinot-core/src/test/java/org/apache/pinot/core/util/SchemaUtilsTest.java
##########
@@ -140,7 +140,7 @@ public void testCompatibilityWithTableConfig() {
         .addDateTime(TIME_COLUMN, DataType.LONG, "1:MILLISECONDS:EPOCH", 
"1:HOURS").build();
     tableConfig = new 
TableConfigBuilder(TableType.REALTIME).setTableName(TABLE_NAME).setTimeColumnName(TIME_COLUMN)
         .setIngestionConfig(
-            new IngestionConfig(null, null, null, Lists.newArrayList(new 
TransformConfig("colA", "round(colB, 1000)"))))
+            new IngestionConfig(null, null, null, Lists.newArrayList(new 
TransformConfig("colA", "round(colB, 1000)")), null))

Review comment:
       Many of the lines in this PR are only for adding null values in the 
IngestionConfig constructor. Can we create a new constructor instead? I think 
multiple constructors should be ok in this case, specially since a lot of the 
arguments are null.




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

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org