cecemei commented on code in PR #18638:
URL: https://github.com/apache/druid/pull/18638#discussion_r2433572845


##########
processing/src/main/java/org/apache/druid/segment/nested/NestedCommonFormatColumnFormatSpec.java:
##########
@@ -40,11 +40,11 @@
  */
 public class NestedCommonFormatColumnFormatSpec
 {
-  private static final NestedCommonFormatColumnFormatSpec DEFAULT =
-      NestedCommonFormatColumnFormatSpec.builder()
-                                        
.setObjectFieldsDictionaryEncoding(StringEncodingStrategy.UTF8_STRATEGY)
-                                        
.setObjectStorageEncoding(ObjectStorageEncoding.SMILE)
-                                        .build();
+  private static final NestedCommonFormatColumnFormatSpec OPERATOR_DEFAULT = 
builder().build();

Review Comment:
   I feel like we should try to explain all these defaults and add some 
documentations on the overriding orders of the spec. The `OPERATOR_DEFAULT` 
seems like a convenient holder instance so that we can call `getEffectiveSpec`, 
it doesn't represent anything meaningful, maybe we should move this to builder 
class, and let builder accept null spec in its constructor. 
   
   Maybe the system default can be moved to indexspec? I see there're some 
duplicates in defining the default for ObjectStorageEncoding and 
ObjectFieldsDictionaryEncoding, i guess its due to indexspec can have all-null 
values column spec as well, so maybe it might be nice to centralize them all in 
one place, like `getObjectFieldsDictionaryEncodingOrDefault`. 



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