Jackie-Jiang commented on code in PR #15006:
URL: https://github.com/apache/pinot/pull/15006#discussion_r1972717022
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/ColumnMetadata.java:
##########
@@ -93,4 +94,6 @@ default boolean isMinMaxValueInvalid() {
Map<IndexType<?, ?, ?>, Long> getIndexSizeMap();
boolean isAutoGenerated();
+
+ List<Object> getChildColumns();
Review Comment:
Please add some javadoc explaining what is child column. Same for other
interface change
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/ForwardIndexConfig.java:
##########
@@ -92,6 +92,7 @@ public static ForwardIndexConfig getDisabled() {
private final String _targetMaxChunkSize;
private final int _targetMaxChunkSizeBytes;
private final int _targetDocsPerChunk;
+ private final Map<String, Object> _mapEncodingConfigs;
Review Comment:
IIUC, map index is a collection of different types of indices. We should
probably add it as a separate index type
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/V1Constants.java:
##########
@@ -127,8 +127,10 @@ public static class Column {
public static final String DATETIME_FORMAT = "datetimeFormat";
public static final String DATETIME_GRANULARITY = "datetimeGranularity";
public static final String COMPLEX_CHILD_FIELD_NAMES =
"complexChildFieldNames";
-
+ public static final String CHILD_COLUMN_METADATA_PROP_FILENAME_SUFFIX =
"child.columnmetadata.properties";
Review Comment:
Don't mix this into the metadata keys. I don't think this is metadata related
##########
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/FieldConfig.java:
##########
@@ -122,7 +122,7 @@ public FieldConfig(@JsonProperty(value = "name", required =
true) String name,
// If null, we will create dictionary encoded forward index by default
public enum EncodingType {
- RAW, DICTIONARY
+ RAW, DICTIONARY, MAP
Review Comment:
I wouldn't count `MAP` as a encoding type. It should be an index type
##########
pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/V1Constants.java:
##########
@@ -127,8 +127,10 @@ public static class Column {
public static final String DATETIME_FORMAT = "datetimeFormat";
public static final String DATETIME_GRANULARITY = "datetimeGranularity";
public static final String COMPLEX_CHILD_FIELD_NAMES =
"complexChildFieldNames";
-
+ public static final String CHILD_COLUMN_METADATA_PROP_FILENAME_SUFFIX =
"child.columnmetadata.properties";
+ public static final String CHILD_COLUMN_METADATA_KEY_SEPARATOR = ".";
public static final String COLUMN_PROPS_KEY_PREFIX = "column.";
+ public static final String CHILDCOLUMNS_PROPS_KEY_PREFIX =
"childcolumns.keys";
Review Comment:
Keep the convention of camel case
--
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: commits-unsubscr...@pinot.apache.org
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
