yupeng9 commented on a change in pull request #6918:
URL: https://github.com/apache/incubator-pinot/pull/6918#discussion_r634062517
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -256,23 +318,73 @@ private void flattenMap(String path, Map<String, Object>
map, Collection<String>
flattenMap(concatName, map, innerMapFields);
}
} else if (value instanceof Collection &&
_unnestFields.contains(concatName)) {
- for (Object inner : (Collection) value) {
- if (inner instanceof Map) {
- Map<String, Object> innerMap = (Map<String, Object>) inner;
- flattenMap(concatName, innerMap, new
ArrayList<>(innerMap.keySet()));
+ Collection collection = (Collection) value;
+ if (_unnestFields.contains(concatName)) {
+ for (Object inner : (Collection) value) {
+ if (inner instanceof Map) {
+ Map<String, Object> innerMap = (Map<String, Object>) inner;
+ flattenMap(concatName, innerMap, new
ArrayList<>(innerMap.keySet()));
+ }
+ }
+ } else if (shallConvertToJson(collection)) {
+ try {
+ // convert the collection to JSON string
+ String jsonString = JsonFunctions.jsonFormat(collection);
+ map.put(field, jsonString);
+ } catch (JsonProcessingException e) {
+ throw new RuntimeException(
+ String.format("Caught exception while converting value to JSON
string %s", value), e);
}
}
- } else if (isArray(value) && _unnestFields.contains(concatName)) {
- for (Object inner : (Object[]) value) {
- if (inner instanceof Map) {
- Map<String, Object> innerMap = (Map<String, Object>) inner;
- flattenMap(concatName, innerMap, new
ArrayList<>(innerMap.keySet()));
+ } else if (isArray(value)) {
+ Object[] array = (Object[]) value;
+ if (_unnestFields.contains(concatName)) {
+ for (Object inner : (Object[]) value) {
+ if (inner instanceof Map) {
+ Map<String, Object> innerMap = (Map<String, Object>) inner;
+ flattenMap(concatName, innerMap, new
ArrayList<>(innerMap.keySet()));
+ }
+ }
+ } else if (shallConvertToJson(array)) {
+ try {
+ // convert the array to JSON string
+ String jsonString = JsonFunctions.jsonFormat(array);
+ map.put(field, jsonString);
+ } catch (JsonProcessingException e) {
+ throw new RuntimeException(
+ String.format("Caught exception while converting value to JSON
string %s", value), e);
}
}
}
}
}
+ private boolean shallConvertToJson(Object[] value) {
+ switch (_collectionToJsonMode) {
+ case ALL:
+ return true;
+ case NONE:
+ return false;
+ case NON_PRIMITIVE:
+ return !containPrimitives(value);
+ default:
+ throw new IllegalArgumentException(String.format("Unsupported
collectionToJsonMode %s", _collectionToJsonMode));
+ }
+ }
+
+ private boolean shallConvertToJson(Collection value) {
Review comment:
I meant `value.toArray()` will create a copy.
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -212,25 +234,65 @@ protected void flattenMap(GenericRow record, List<String>
columns) {
}
}
flattenMap(record, mapColumns);
- } else if (value instanceof Collection &&
_unnestFields.contains(column)) {
- for (Object inner : (Collection) value) {
- if (inner instanceof Map) {
- Map<String, Object> innerMap = (Map<String, Object>) inner;
- flattenMap(column, innerMap, new ArrayList<>(innerMap.keySet()));
+ } else if (value instanceof Collection) {
+ Collection collection = (Collection) value;
+ if (_unnestFields.contains(column)) {
+ for (Object inner : collection) {
+ if (inner instanceof Map) {
+ Map<String, Object> innerMap = (Map<String, Object>) inner;
+ flattenMap(column, innerMap, new ArrayList<>(innerMap.keySet()));
+ }
+ }
+ } else if (shallConvertToJson(collection)) {
+ try {
+ // convert the collection to JSON string
+ String jsonString = JsonFunctions.jsonFormat(collection);
+ record.putValue(column, jsonString);
+ } catch (JsonProcessingException e) {
+ throw new RuntimeException(
+ String.format("Caught exception while converting value to JSON
string %s", value), e);
}
}
- } else if (isArray(value) && _unnestFields.contains(column)) {
- for (Object inner : (Object[]) value) {
- if (inner instanceof Map) {
- Map<String, Object> innerMap = (Map<String, Object>) inner;
- flattenMap(column, innerMap, new ArrayList<>(innerMap.keySet()));
+ } else if (isArray(value)) {
+ Object[] array = (Object[]) value;
+ if (_unnestFields.contains(column)) {
+ for (Object inner : array) {
+ if (inner instanceof Map) {
+ Map<String, Object> innerMap = (Map<String, Object>) inner;
+ flattenMap(column, innerMap, new ArrayList<>(innerMap.keySet()));
+ }
+ }
+ } else if (shallConvertToJson(array)) {
+ try {
+ // convert the array to JSON string
+ String jsonString = JsonFunctions.jsonFormat(array);
+ record.putValue(column, jsonString);
+ } catch (JsonProcessingException e) {
+ throw new RuntimeException(
+ String.format("Caught exception while converting value to JSON
string %s", value), e);
}
}
}
}
}
- static private boolean isArray(Object obj) {
+ private boolean containPrimitives(Object[] value) {
+ if (value.length == 0) {
+ return true;
+ }
+ Object element = value[0];
+ return !(element instanceof Map || element instanceof Collection ||
isArray(element));
+ }
+
+ private boolean containPrimitives(Collection value) {
+ if (value.isEmpty()) {
+ return true;
+ }
+ Object element = value.iterator().next();
+ return !(element instanceof Map || element instanceof Collection ||
isArray(element));
+ }
Review comment:
Could be inlined. But this makes the code slightly readable given the
negation is used in `shallConvertToJson `
##########
File path:
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java
##########
@@ -78,17 +81,27 @@
*/
public class ComplexTypeTransformer implements RecordTransformer {
private static final String DEFAULT_DELIMITER = ".";
+ public static final ComplexTypeConfig.CollectionToJsonMode
DEFAULT_COLLECTION_TO_JSON_MODE =
+ ComplexTypeConfig.CollectionToJsonMode.NON_PRIMITIVE;
private final List<String> _unnestFields;
private final String _delimiter;
+ private final ComplexTypeConfig.CollectionToJsonMode _collectionToJsonMode;
public ComplexTypeTransformer(TableConfig tableConfig) {
- this(parseUnnestFields(tableConfig), parseDelimiter(tableConfig));
+ this(parseUnnestFields(tableConfig), parseDelimiter(tableConfig),
parseCollectionToJsonMode(tableConfig));
Review comment:
I feel it's not that bad. Usually I do what you suggest, but in this one
there's some other state change in the constructor.
--
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
