Jackie-Jiang commented on code in PR #13905:
URL: https://github.com/apache/pinot/pull/13905#discussion_r1736752128
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -613,8 +616,19 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
properties.setProperty(getKeyFor(column, DATETIME_GRANULARITY),
dateTimeFieldSpec.getGranularity());
}
+ // complex field
+ if (fieldSpec.getFieldType().equals(FieldType.COMPLEX)) {
Review Comment:
(nit) Same for other enum checks
```suggestion
if (fieldSpec.getFieldType() == FieldType.COMPLEX) {
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -633,6 +647,55 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
}
}
+ /**
+ * In order to persist complex field metadata, we need to recursively add
child field specs
+ * So, each complex field spec will have a property for its child field
names and each child field will have its
+ * own properties of the detailed field spec.
+ * E.g. a COMPLEX type `intMap` of Map<String, Integer> has 2 child fields:
+ * - key in STRING type and value in INT type.
+ * Then we will have the following properties to define a COMPLEX field:
+ * column.intMap.childFieldNames = [key, value]
+ * column.intMap$$key.columnType = DIMENSION
+ * column.intMap$$key.dataType = STRING
+ * column.intMap$$key.isSingleValued = true
+ * column.intMap$$value.columnType = DIMENSION
+ * column.intMap$$value.dataType = INT
+ * column.intMap$$value.isSingleValued = true
+ */
+ public static void addFieldSpec(PropertiesConfiguration properties, String
column, FieldSpec fieldSpec) {
Review Comment:
Do we have enough info stored to load the index? In order to load
dictionary, I believe we need to store cardinality, bits per element and other
dictionary related info
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -633,6 +647,55 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
}
}
+ /**
+ * In order to persist complex field metadata, we need to recursively add
child field specs
+ * So, each complex field spec will have a property for its child field
names and each child field will have its
+ * own properties of the detailed field spec.
+ * E.g. a COMPLEX type `intMap` of Map<String, Integer> has 2 child fields:
+ * - key in STRING type and value in INT type.
+ * Then we will have the following properties to define a COMPLEX field:
+ * column.intMap.childFieldNames = [key, value]
+ * column.intMap$$key.columnType = DIMENSION
+ * column.intMap$$key.dataType = STRING
+ * column.intMap$$key.isSingleValued = true
+ * column.intMap$$value.columnType = DIMENSION
+ * column.intMap$$value.dataType = INT
+ * column.intMap$$value.isSingleValued = true
+ */
+ public static void addFieldSpec(PropertiesConfiguration properties, String
column, FieldSpec fieldSpec) {
+ properties.setProperty(getKeyFor(column, COLUMN_TYPE),
String.valueOf(fieldSpec.getFieldType()));
+ if (!column.equals(fieldSpec.getName())) {
+ properties.setProperty(getKeyFor(column, COLUMN_NAME),
String.valueOf(fieldSpec.getName()));
Review Comment:
Do we ever allow this? Will complex type has a different field name under
the key?
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -613,8 +616,19 @@ public static void
addColumnMetadataInfo(PropertiesConfiguration properties, Str
properties.setProperty(getKeyFor(column, DATETIME_GRANULARITY),
dateTimeFieldSpec.getGranularity());
}
+ // complex field
+ if (fieldSpec.getFieldType().equals(FieldType.COMPLEX)) {
+ ComplexFieldSpec complexFieldSpec = (ComplexFieldSpec) fieldSpec;
+ properties.setProperty(getKeyFor(column, COMPLEX_CHILD_FIELD_NAMES),
+ new ArrayList<>(complexFieldSpec.getChildFieldSpecs().keySet()));
+ for (Map.Entry<String, FieldSpec> entry :
complexFieldSpec.getChildFieldSpecs().entrySet()) {
+ addFieldSpec(properties, ComplexFieldSpec.getFullChildName(column,
entry.getKey()), entry.getValue());
+ }
+ }
+
+ addFieldSpec(properties, column, fieldSpec);
Review Comment:
I'm a little bit confused. Are we adding the same properties again?
--
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]
