[GitHub] [incubator-pinot] Jackie-Jiang commented on a change in pull request #6905: Allow overriding delimiter in Complex type config

[GitBox]

Jackie-Jiang commented on a change in pull request #6905:
URL: https://github.com/apache/incubator-pinot/pull/6905#discussion_r631452485



##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -78,27 +78,39 @@
  */
 public class ComplexTypeTransformer implements RecordTransformer {
   // TODO: make configurable
-  private static final CharSequence DELIMITER = ".";
+  private static final CharSequence DEFAULT_DELIMITER = ".";
   private final List<String> _unnestFields;
+  private final CharSequence _delimiter;

Review comment:
       Change it to `String`

##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -78,27 +78,39 @@
  */
 public class ComplexTypeTransformer implements RecordTransformer {
   // TODO: make configurable
-  private static final CharSequence DELIMITER = ".";
+  private static final CharSequence DEFAULT_DELIMITER = ".";

Review comment:
       Change it to `String`

##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -78,27 +78,39 @@
  */
 public class ComplexTypeTransformer implements RecordTransformer {
   // TODO: make configurable
-  private static final CharSequence DELIMITER = ".";
+  private static final CharSequence DEFAULT_DELIMITER = ".";
   private final List<String> _unnestFields;
+  private final CharSequence _delimiter;
 
   public ComplexTypeTransformer(TableConfig tableConfig) {
-    if (tableConfig.getIngestionConfig() != null && 
tableConfig.getIngestionConfig().getComplexTypeConfig() != null) {
-      _unnestFields = 
tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestFields() != 
null ? tableConfig
-          .getIngestionConfig().getComplexTypeConfig().getUnnestFields() : new 
ArrayList<>();
-      // the unnest fields are sorted to achieve the topological sort of the 
collections, so that the parent collection
-      // (e.g. foo) is unnested before the child collection (e.g. foo.bar)
-      Collections.sort(_unnestFields);
-    } else {
-      _unnestFields = new ArrayList<>();
-    }
+    this(parseUnnestFields(tableConfig), parseDelimiter(tableConfig));
   }
 
   @VisibleForTesting
-  public ComplexTypeTransformer(List<String> unnestFields) {
+  public ComplexTypeTransformer(List<String> unnestFields, CharSequence 
delimiter) {
     _unnestFields = new ArrayList<>(unnestFields);
+    _delimiter = delimiter;
     Collections.sort(_unnestFields);

Review comment:
       Move the comments about topological sort here

##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -78,27 +78,39 @@
  */
 public class ComplexTypeTransformer implements RecordTransformer {
   // TODO: make configurable
-  private static final CharSequence DELIMITER = ".";
+  private static final CharSequence DEFAULT_DELIMITER = ".";
   private final List<String> _unnestFields;
+  private final CharSequence _delimiter;
 
   public ComplexTypeTransformer(TableConfig tableConfig) {
-    if (tableConfig.getIngestionConfig() != null && 
tableConfig.getIngestionConfig().getComplexTypeConfig() != null) {
-      _unnestFields = 
tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestFields() != 
null ? tableConfig
-          .getIngestionConfig().getComplexTypeConfig().getUnnestFields() : new 
ArrayList<>();
-      // the unnest fields are sorted to achieve the topological sort of the 
collections, so that the parent collection
-      // (e.g. foo) is unnested before the child collection (e.g. foo.bar)
-      Collections.sort(_unnestFields);
-    } else {
-      _unnestFields = new ArrayList<>();
-    }
+    this(parseUnnestFields(tableConfig), parseDelimiter(tableConfig));
   }
 
   @VisibleForTesting
-  public ComplexTypeTransformer(List<String> unnestFields) {
+  public ComplexTypeTransformer(List<String> unnestFields, CharSequence 
delimiter) {

Review comment:
       ```suggestion
     public ComplexTypeTransformer(List<String> unnestFields, String delimiter) 
{
   ```




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