Jackie-Jiang commented on a change in pull request #8273:
URL: https://github.com/apache/pinot/pull/8273#discussion_r825474664
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -89,25 +90,30 @@
private final List<String> _fieldsToUnnest;
private final String _delimiter;
private final ComplexTypeConfig.CollectionNotUnnestedToJson
_collectionNotUnnestedToJson;
+ private final Map<String, String> _prefixesToRename;
public ComplexTypeTransformer(TableConfig tableConfig) {
- this(parseFieldsToUnnest(tableConfig), parseDelimiter(tableConfig),
parseCollectionNotUnnestedToJson(tableConfig));
+ this(parseFieldsToUnnest(tableConfig), parseDelimiter(tableConfig),
+ parseCollectionNotUnnestedToJson(tableConfig),
parsePrefixesToRename(tableConfig));
}
@VisibleForTesting
ComplexTypeTransformer(List<String> fieldsToUnnest, String delimiter) {
- this(fieldsToUnnest, delimiter, DEFAULT_COLLECTION_TO_JSON_MODE);
+ this(fieldsToUnnest, delimiter, DEFAULT_COLLECTION_TO_JSON_MODE, new
HashMap<>());
}
@VisibleForTesting
- ComplexTypeTransformer(List<String> fieldsToUnnest, String delimiter,
- ComplexTypeConfig.CollectionNotUnnestedToJson
collectionNotUnnestedToJson) {
+ ComplexTypeTransformer(List<String> fieldsToUnnest,
Review comment:
(minor, code format) keep the Pinot Style format
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -89,25 +90,30 @@
private final List<String> _fieldsToUnnest;
private final String _delimiter;
private final ComplexTypeConfig.CollectionNotUnnestedToJson
_collectionNotUnnestedToJson;
+ private final Map<String, String> _prefixesToRename;
public ComplexTypeTransformer(TableConfig tableConfig) {
- this(parseFieldsToUnnest(tableConfig), parseDelimiter(tableConfig),
parseCollectionNotUnnestedToJson(tableConfig));
+ this(parseFieldsToUnnest(tableConfig), parseDelimiter(tableConfig),
+ parseCollectionNotUnnestedToJson(tableConfig),
parsePrefixesToRename(tableConfig));
}
@VisibleForTesting
ComplexTypeTransformer(List<String> fieldsToUnnest, String delimiter) {
- this(fieldsToUnnest, delimiter, DEFAULT_COLLECTION_TO_JSON_MODE);
+ this(fieldsToUnnest, delimiter, DEFAULT_COLLECTION_TO_JSON_MODE, new
HashMap<>());
}
@VisibleForTesting
- ComplexTypeTransformer(List<String> fieldsToUnnest, String delimiter,
- ComplexTypeConfig.CollectionNotUnnestedToJson
collectionNotUnnestedToJson) {
+ ComplexTypeTransformer(List<String> fieldsToUnnest,
+ String delimiter,
+ ComplexTypeConfig.CollectionNotUnnestedToJson
collectionNotUnnestedToJson,
+ Map<String, String> prefixesToRename) {
_fieldsToUnnest = new ArrayList<>(fieldsToUnnest);
_delimiter = delimiter;
_collectionNotUnnestedToJson = collectionNotUnnestedToJson;
// 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(_fieldsToUnnest);
+ _prefixesToRename = new HashMap<>(prefixesToRename);
Review comment:
Don't copy a new map since it will create unnecessary garbage
##########
File path:
pinot-spi/src/main/java/org/apache/pinot/spi/config/table/ingestion/ComplexTypeConfig.java
##########
@@ -44,13 +45,18 @@
@JsonPropertyDescription("The mode of converting collection to JSON string")
private final CollectionNotUnnestedToJson _collectionNotUnnestedToJson;
+ @JsonPropertyDescription("Map of <prefix, replacement> so matching fields
are renamed to start with the replacement")
+ private final Map<String, String> _prefixesToRename;
+
@JsonCreator
public ComplexTypeConfig(@JsonProperty("fieldsToUnnest") @Nullable
List<String> fieldsToUnnest,
@JsonProperty("delimiter") @Nullable String delimiter,
- @JsonProperty("collectionNotUnnestedToJson") @Nullable
CollectionNotUnnestedToJson collectionNotUnnestedToJson) {
+ @JsonProperty("collectionNotUnnestedToJson") @Nullable
CollectionNotUnnestedToJson collectionNotUnnestedToJson,
+ @JsonProperty("prefixesToRename") @Nullable
Map<String, String> prefixesToRename) {
Review comment:
(code format) reformat this
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -89,25 +90,30 @@
private final List<String> _fieldsToUnnest;
private final String _delimiter;
private final ComplexTypeConfig.CollectionNotUnnestedToJson
_collectionNotUnnestedToJson;
+ private final Map<String, String> _prefixesToRename;
public ComplexTypeTransformer(TableConfig tableConfig) {
- this(parseFieldsToUnnest(tableConfig), parseDelimiter(tableConfig),
parseCollectionNotUnnestedToJson(tableConfig));
+ this(parseFieldsToUnnest(tableConfig), parseDelimiter(tableConfig),
+ parseCollectionNotUnnestedToJson(tableConfig),
parsePrefixesToRename(tableConfig));
}
@VisibleForTesting
ComplexTypeTransformer(List<String> fieldsToUnnest, String delimiter) {
- this(fieldsToUnnest, delimiter, DEFAULT_COLLECTION_TO_JSON_MODE);
+ this(fieldsToUnnest, delimiter, DEFAULT_COLLECTION_TO_JSON_MODE, new
HashMap<>());
Review comment:
(minor)
```suggestion
this(fieldsToUnnest, delimiter, DEFAULT_COLLECTION_TO_JSON_MODE,
Collections.emptyMap());
```
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -279,6 +295,31 @@ protected void flattenMap(GenericRow record, List<String>
columns) {
}
}
+ /**
+ * Loops through all columns and renames the column's prefix with the
corresponding replacement if the prefix matches.
+ */
+ @VisibleForTesting
+ protected void renamePrefixes(GenericRow record) {
+ if (_prefixesToRename.isEmpty()) {
+ return;
+ }
+ List<String> fields = new
ArrayList<>(record.getFieldToValueMap().keySet());
Review comment:
Avoid creating this extra list
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
##########
@@ -348,6 +349,23 @@ public static void validateIngestionConfig(TableConfig
tableConfig, @Nullable Sc
}
}
}
+
+ // Complex configs
+ ComplexTypeConfig complexTypeConfig =
ingestionConfig.getComplexTypeConfig();
+ if (complexTypeConfig != null && schema != null) {
+ Map<String, String> prefixesToRename =
complexTypeConfig.getPrefixesToRename();
+ Set<String> fieldNames = schema.getFieldSpecMap().keySet();
+ if (prefixesToRename != null) {
+ for (String prefix : prefixesToRename.keySet()) {
+ for (String field : fieldNames) {
+ if (field.startsWith(prefix)) {
Review comment:
(minor) Use `Preconditions.checkState()` with exception message template
to simplify this for readability
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java
##########
@@ -348,6 +349,23 @@ public static void validateIngestionConfig(TableConfig
tableConfig, @Nullable Sc
}
}
}
+
+ // Complex configs
+ ComplexTypeConfig complexTypeConfig =
ingestionConfig.getComplexTypeConfig();
+ if (complexTypeConfig != null && schema != null) {
+ Map<String, String> prefixesToRename =
complexTypeConfig.getPrefixesToRename();
+ Set<String> fieldNames = schema.getFieldSpecMap().keySet();
+ if (prefixesToRename != null) {
Review comment:
```suggestion
if (MapUtils.isNotEmpty(prefixesToRename)) {
```
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -149,12 +155,22 @@ public static ComplexTypeTransformer
getComplexTypeTransformer(TableConfig table
}
}
+ private static Map<String, String> parsePrefixesToRename(TableConfig
tableConfig) {
+ if (tableConfig.getIngestionConfig() != null &&
tableConfig.getIngestionConfig().getComplexTypeConfig() != null
+ &&
tableConfig.getIngestionConfig().getComplexTypeConfig().getPrefixesToRename()
!= null) {
+ return
tableConfig.getIngestionConfig().getComplexTypeConfig().getPrefixesToRename();
+ } else {
+ return new HashMap<>();
Review comment:
(minor)
```suggestion
return Collections.emptyMap();
```
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -279,6 +295,31 @@ protected void flattenMap(GenericRow record, List<String>
columns) {
}
}
+ /**
+ * Loops through all columns and renames the column's prefix with the
corresponding replacement if the prefix matches.
+ */
+ @VisibleForTesting
+ protected void renamePrefixes(GenericRow record) {
+ if (_prefixesToRename.isEmpty()) {
+ return;
+ }
+ List<String> fields = new
ArrayList<>(record.getFieldToValueMap().keySet());
+ for (String prefix : _prefixesToRename.keySet()) {
Review comment:
Use `.entrySet()` to avoid the extra map lookups on line 312
--
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]
