Jackie-Jiang commented on a change in pull request #6845: URL: https://github.com/apache/incubator-pinot/pull/6845#discussion_r630460483
########## File path: pinot-common/src/test/java/org/apache/pinot/common/utils/config/TableConfigSerDeTest.java ########## @@ -273,14 +274,17 @@ public void testSerDe() streamConfigMaps.add(streamConfigMap); List<Map<String, String>> batchConfigMaps = new ArrayList<>(); batchConfigMaps.add(batchConfigMap); + List<String> unnestConfig = Arrays.asList("c1, c2"); IngestionConfig ingestionConfig = new IngestionConfig(new BatchIngestionConfig(batchConfigMaps, "APPEND", "HOURLY"), - new StreamIngestionConfig(streamConfigMaps), new FilterConfig("filterFunc(foo)"), transformConfigs); + new StreamIngestionConfig(streamConfigMaps), new FilterConfig("filterFunc(foo)"), transformConfigs, + new ComplexTypeConfig(unnestConfig)); TableConfig tableConfig = tableConfigBuilder.setIngestionConfig(ingestionConfig).build(); checkIngestionConfig(tableConfig); // Serialize then de-serialize + Review comment: (nit) remove ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,252 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; + private final List<String> _unnestFields; + + public ComplexTypeTransformer(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null) { + _unnestFields = tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestConfig() != null ? tableConfig + .getIngestionConfig().getComplexTypeConfig().getUnnestConfig() : new ArrayList<>(); + } else { + _unnestFields = new ArrayList<>(); + } + } + + @VisibleForTesting + public ComplexTypeTransformer(List<String> unnestCollections) { + _unnestFields = new ArrayList<>(unnestCollections); + Collections.sort(_unnestFields); Review comment: Add some comments on why we need to sort, and why alphabetical sort is equivalent to topology sort ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,252 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; + private final List<String> _unnestFields; + + public ComplexTypeTransformer(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null) { + _unnestFields = tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestConfig() != null ? tableConfig + .getIngestionConfig().getComplexTypeConfig().getUnnestConfig() : new ArrayList<>(); + } else { + _unnestFields = new ArrayList<>(); + } + } + + @VisibleForTesting + public ComplexTypeTransformer(List<String> unnestCollections) { + _unnestFields = new ArrayList<>(unnestCollections); + Collections.sort(_unnestFields); + } + + public static boolean isComplexTypeHandlingEnabled(TableConfig tableConfig) { + return tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null; + } + + @Nullable + @Override + public GenericRow transform(GenericRow record) { + flattenMap(record, new ArrayList<>(record.getFieldToValueMap().keySet())); + for (String collection : _unnestFields) { + unnestCollection(record, collection); + } + return record; + } + + private GenericRow unnestCollection(GenericRow record, String column) { + if (record.getValue(GenericRow.MULTIPLE_RECORDS_KEY) == null) { + List<GenericRow> list = new ArrayList<>(); + unnestCollection(record, column, list); + record.putValue(GenericRow.MULTIPLE_RECORDS_KEY, list); + } else { + Collection<GenericRow> records = (Collection) record.getValue(GenericRow.MULTIPLE_RECORDS_KEY); + List<GenericRow> list = new ArrayList<>(); + for (GenericRow innerRecord : records) { + unnestCollection(innerRecord, column, list); + } + record.putValue(GenericRow.MULTIPLE_RECORDS_KEY, list); + } + return record; + } + + private void unnestCollection(GenericRow record, String column, List<GenericRow> list) { + Object value = record.removeValue(column); + if (value == null) { + // use the record itself + list.add(record); + } else if (value instanceof Collection) { + if (((Collection) value).isEmpty()) { + // use the record itself + list.add(record); + } else { + for (Object obj : (Collection) value) { + GenericRow copy = flattenCollectionItem(record, obj, column); + list.add(copy); + } + } + } else if (isArray(value)) { + if (((Object[]) value).length == 0) { + // use the record itself + list.add(record); + } else { + for (Object obj : (Object[]) value) { + GenericRow copy = flattenCollectionItem(record, obj, column); + list.add(copy); + } + } + } + } + + private GenericRow flattenCollectionItem(GenericRow record, Object obj, String column) { + GenericRow copy = record.copy(); + if (obj instanceof Map) { + Map<String, Object> map = (Map<String, Object>) obj; + for (Map.Entry<String, Object> entry : map.entrySet()) { + String flattenName = concat(column, entry.getKey()); + copy.putValue(flattenName, entry.getValue()); + } + } else { Review comment: Should we also handle multi-dimensional array here? You may pass in the list, and make it recursive ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,252 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; + private final List<String> _unnestFields; + + public ComplexTypeTransformer(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null) { + _unnestFields = tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestConfig() != null ? tableConfig + .getIngestionConfig().getComplexTypeConfig().getUnnestConfig() : new ArrayList<>(); Review comment: We also need to sort here ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,252 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; + private final List<String> _unnestFields; + + public ComplexTypeTransformer(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null) { + _unnestFields = tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestConfig() != null ? tableConfig + .getIngestionConfig().getComplexTypeConfig().getUnnestConfig() : new ArrayList<>(); + } else { + _unnestFields = new ArrayList<>(); + } + } + + @VisibleForTesting + public ComplexTypeTransformer(List<String> unnestCollections) { + _unnestFields = new ArrayList<>(unnestCollections); + Collections.sort(_unnestFields); + } + + public static boolean isComplexTypeHandlingEnabled(TableConfig tableConfig) { + return tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null; + } + + @Nullable + @Override + public GenericRow transform(GenericRow record) { + flattenMap(record, new ArrayList<>(record.getFieldToValueMap().keySet())); + for (String collection : _unnestFields) { + unnestCollection(record, collection); + } + return record; + } + + private GenericRow unnestCollection(GenericRow record, String column) { + if (record.getValue(GenericRow.MULTIPLE_RECORDS_KEY) == null) { + List<GenericRow> list = new ArrayList<>(); + unnestCollection(record, column, list); + record.putValue(GenericRow.MULTIPLE_RECORDS_KEY, list); + } else { + Collection<GenericRow> records = (Collection) record.getValue(GenericRow.MULTIPLE_RECORDS_KEY); + List<GenericRow> list = new ArrayList<>(); + for (GenericRow innerRecord : records) { + unnestCollection(innerRecord, column, list); + } + record.putValue(GenericRow.MULTIPLE_RECORDS_KEY, list); + } + return record; + } + + private void unnestCollection(GenericRow record, String column, List<GenericRow> list) { + Object value = record.removeValue(column); + if (value == null) { + // use the record itself + list.add(record); + } else if (value instanceof Collection) { + if (((Collection) value).isEmpty()) { + // use the record itself + list.add(record); + } else { + for (Object obj : (Collection) value) { + GenericRow copy = flattenCollectionItem(record, obj, column); + list.add(copy); + } + } + } else if (isArray(value)) { + if (((Object[]) value).length == 0) { + // use the record itself + list.add(record); + } else { + for (Object obj : (Object[]) value) { + GenericRow copy = flattenCollectionItem(record, obj, column); + list.add(copy); + } + } + } + } + + private GenericRow flattenCollectionItem(GenericRow record, Object obj, String column) { + GenericRow copy = record.copy(); + if (obj instanceof Map) { + Map<String, Object> map = (Map<String, Object>) obj; + for (Map.Entry<String, Object> entry : map.entrySet()) { + String flattenName = concat(column, entry.getKey()); + copy.putValue(flattenName, entry.getValue()); + } + } else { + copy.putValue(column, obj); + } + return copy; + } + + /** + * Recursively flatten all the Maps in the record. It will also navigate into the collections marked as "unnest" and + * flatten the nested maps. + */ + @VisibleForTesting + protected void flattenMap(GenericRow record, Collection<String> columns) { + for (String column : columns) { + Object value = record.getValue(column); + if (value instanceof Map) { + Map<String, Object> map = (Map) record.removeValue(column); + List<String> mapColumns = new ArrayList<>(); + for (Map.Entry<String, Object> entry : new ArrayList<>(map.entrySet())) { + String flattenName = concat(column, entry.getKey()); + Object nestedValue = entry.getValue(); + record.putValue(flattenName, nestedValue); + if (nestedValue instanceof Map || nestedValue instanceof Collection || isArray(nestedValue)) { + mapColumns.add(flattenName); + } + } + flattenMap(record, mapColumns); + } else if (value instanceof Collection && _unnestFields.contains(column)) { + for (Object inner : (Collection) record.getValue(column)) { Review comment: ```suggestion for (Object inner : (Collection) value) { ``` ########## File path: pinot-spi/src/main/java/org/apache/pinot/spi/config/table/ingestion/ComplexTypeConfig.java ########## @@ -0,0 +1,46 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.spi.config.table.ingestion; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyDescription; +import java.util.List; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.BaseJsonConfig; + + +/** + * Config related to handling complex type + */ +public class ComplexTypeConfig extends BaseJsonConfig { + + @JsonPropertyDescription("The collections to unnest") Review comment: `"The fields to unnest" ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,252 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; + private final List<String> _unnestFields; + + public ComplexTypeTransformer(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null) { + _unnestFields = tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestConfig() != null ? tableConfig + .getIngestionConfig().getComplexTypeConfig().getUnnestConfig() : new ArrayList<>(); + } else { + _unnestFields = new ArrayList<>(); + } + } + + @VisibleForTesting + public ComplexTypeTransformer(List<String> unnestCollections) { Review comment: `unnestFields` ########## File path: pinot-spi/src/main/java/org/apache/pinot/spi/config/table/ingestion/ComplexTypeConfig.java ########## @@ -0,0 +1,46 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.spi.config.table.ingestion; + +import com.fasterxml.jackson.annotation.JsonCreator; +import com.fasterxml.jackson.annotation.JsonProperty; +import com.fasterxml.jackson.annotation.JsonPropertyDescription; +import java.util.List; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.BaseJsonConfig; + + +/** + * Config related to handling complex type + */ +public class ComplexTypeConfig extends BaseJsonConfig { + + @JsonPropertyDescription("The collections to unnest") + private final List<String> _unnestConfig; Review comment: `_unnestFields` ########## File path: pinot-common/src/test/java/org/apache/pinot/common/utils/config/TableConfigSerDeTest.java ########## @@ -273,14 +274,17 @@ public void testSerDe() streamConfigMaps.add(streamConfigMap); List<Map<String, String>> batchConfigMaps = new ArrayList<>(); batchConfigMaps.add(batchConfigMap); + List<String> unnestConfig = Arrays.asList("c1, c2"); Review comment: `unnestFields` ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,252 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; + private final List<String> _unnestFields; + + public ComplexTypeTransformer(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null) { + _unnestFields = tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestConfig() != null ? tableConfig + .getIngestionConfig().getComplexTypeConfig().getUnnestConfig() : new ArrayList<>(); + } else { + _unnestFields = new ArrayList<>(); + } + } + + @VisibleForTesting + public ComplexTypeTransformer(List<String> unnestCollections) { + _unnestFields = new ArrayList<>(unnestCollections); + Collections.sort(_unnestFields); + } + + public static boolean isComplexTypeHandlingEnabled(TableConfig tableConfig) { + return tableConfig.getIngestionConfig() != null && tableConfig.getIngestionConfig().getComplexTypeConfig() != null; + } + + @Nullable + @Override + public GenericRow transform(GenericRow record) { + flattenMap(record, new ArrayList<>(record.getFieldToValueMap().keySet())); + for (String collection : _unnestFields) { + unnestCollection(record, collection); + } + return record; + } + + private GenericRow unnestCollection(GenericRow record, String column) { + if (record.getValue(GenericRow.MULTIPLE_RECORDS_KEY) == null) { + List<GenericRow> list = new ArrayList<>(); + unnestCollection(record, column, list); + record.putValue(GenericRow.MULTIPLE_RECORDS_KEY, list); + } else { + Collection<GenericRow> records = (Collection) record.getValue(GenericRow.MULTIPLE_RECORDS_KEY); + List<GenericRow> list = new ArrayList<>(); + for (GenericRow innerRecord : records) { + unnestCollection(innerRecord, column, list); + } + record.putValue(GenericRow.MULTIPLE_RECORDS_KEY, list); + } + return record; + } + + private void unnestCollection(GenericRow record, String column, List<GenericRow> list) { + Object value = record.removeValue(column); + if (value == null) { + // use the record itself + list.add(record); + } else if (value instanceof Collection) { + if (((Collection) value).isEmpty()) { + // use the record itself + list.add(record); + } else { + for (Object obj : (Collection) value) { + GenericRow copy = flattenCollectionItem(record, obj, column); + list.add(copy); + } + } + } else if (isArray(value)) { + if (((Object[]) value).length == 0) { + // use the record itself + list.add(record); + } else { + for (Object obj : (Object[]) value) { + GenericRow copy = flattenCollectionItem(record, obj, column); + list.add(copy); + } + } + } + } + + private GenericRow flattenCollectionItem(GenericRow record, Object obj, String column) { + GenericRow copy = record.copy(); + if (obj instanceof Map) { + Map<String, Object> map = (Map<String, Object>) obj; + for (Map.Entry<String, Object> entry : map.entrySet()) { + String flattenName = concat(column, entry.getKey()); + copy.putValue(flattenName, entry.getValue()); + } + } else { + copy.putValue(column, obj); + } + return copy; + } + + /** + * Recursively flatten all the Maps in the record. It will also navigate into the collections marked as "unnest" and + * flatten the nested maps. + */ + @VisibleForTesting + protected void flattenMap(GenericRow record, Collection<String> columns) { Review comment: ```suggestion protected void flattenMap(GenericRow record, List<String> columns) { ``` ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,229 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.config.table.ingestion.ComplexTypeConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; Review comment: Let's add TODOs for issues that will be addressed in separate PRs so that they are always tracked ########## File path: pinot-core/src/main/java/org/apache/pinot/core/data/manager/realtime/LLRealtimeSegmentDataManager.java ########## @@ -483,6 +485,9 @@ private void processStreamEvents(MessageBatch messagesAndOffsets, long idlePipeS messagesAndOffsets.getMessageLengthAtIndex(index), reuse); if (decodedRow != null) { try { + if (_complexTypeTransformer != null) { + decodedRow = _complexTypeTransformer.transform(decodedRow); Review comment: Ideally we should put the `ComplexTypeTransformer` as part of the `CompositeTransformer`, but it might be hard with the current implementation as the interface doesn't consider generating multiple records from one. Can you please add a TODO here so that we can merge them in the future. Also, because it is not part of the `CompositeTransformer`, you might need to add the same logic into the offline segment creation code to support complex type for batch ingestion ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,229 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.config.table.ingestion.ComplexTypeConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; + private final List<String> _collectionsToUnnest; + + public ComplexTypeTransformer(TableConfig tableConfig) { Review comment: Add a TODO ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,229 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.config.table.ingestion.ComplexTypeConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; + private final List<String> _collectionsToUnnest; + + public ComplexTypeTransformer(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() != null + && tableConfig.getIngestionConfig().getComplexTypeConfig() != null) { + _collectionsToUnnest = + tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestConfig() != null ? tableConfig + .getIngestionConfig().getComplexTypeConfig().getUnnestConfig() : new ArrayList<>(); + } else { + _collectionsToUnnest = new ArrayList<>(); + } + } + + @VisibleForTesting + public ComplexTypeTransformer(List<String> unnestCollections) { + _collectionsToUnnest = new ArrayList<>(unnestCollections); + Collections.sort(_collectionsToUnnest); + } + + public static boolean isComplexTypeHandlingEnabled(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() == null + || tableConfig.getIngestionConfig().getComplexTypeConfig() == null + || tableConfig.getIngestionConfig().getComplexTypeConfig().getMode() == null) { + return false; + } + return tableConfig.getIngestionConfig().getComplexTypeConfig().getMode() + != ComplexTypeConfig.Mode.NONE; + } + + @Nullable + @Override + public GenericRow transform(GenericRow record) { + flattenMap(record, new HashSet<>(record.getFieldToValueMap().keySet())); + for (String collection : _collectionsToUnnest) { + unnestCollection(record, collection); + } + return record; + } + + private GenericRow unnestCollection(GenericRow record, String column) { + if (record.getValue(GenericRow.MULTIPLE_RECORDS_KEY) == null) { + List<GenericRow> list = new ArrayList<>(); + unnestCollection(record, column, list); + record.putValue(GenericRow.MULTIPLE_RECORDS_KEY, list); + } else { + Collection<GenericRow> records = (Collection) record.getValue(GenericRow.MULTIPLE_RECORDS_KEY); + List<GenericRow> list = new ArrayList<>(); + for (GenericRow innerRecord : records) { + unnestCollection(innerRecord, column, list); + } + record.putValue(GenericRow.MULTIPLE_RECORDS_KEY, list); + } + return record; + } + + private void unnestCollection(GenericRow record, String column, List<GenericRow> list) { + Object value = record.removeValue(column); + if (value == null) { + // use the record itself + list.add(record); + } else if (value instanceof Collection) { + if (((Collection) value).isEmpty()) { + // use the record itself + list.add(record); + } else { + for (Object obj : (Collection) value) { + GenericRow copy = flattenCollectionItem(record, obj, column); + list.add(copy); + } + } + } else if (value.getClass().isArray()) { + if (((Object[]) value).length == 0) { + // use the record itself + list.add(record); + } else { + for (Object obj : (Object[]) value) { + GenericRow copy = flattenCollectionItem(record, obj, column); + list.add(copy); + } + } + } + } + + private GenericRow flattenCollectionItem(GenericRow record, Object obj, String column) { + GenericRow copy = record.copy(); + if (obj instanceof Map) { + Map<String, Object> map = (Map<String, Object>) obj; + for (Map.Entry<String, Object> entry : new HashSet<>(map.entrySet())) { + String flattenName = concat(column, entry.getKey()); + copy.putValue(flattenName, entry.getValue()); + } + } else { + copy.putValue(column, obj); + } + return copy; + } + + @VisibleForTesting + protected GenericRow flattenMap(GenericRow record, Collection<String> columns) { + for (String column : columns) { + if (record.getValue(column) instanceof Map) { + Map<String, Object> map = (Map) record.removeValue(column); + List<String> mapColumns = new ArrayList<>(); + for (Map.Entry<String, Object> entry : new HashSet<>(map.entrySet())) { + String flattenName = concat(column, entry.getKey()); + record.putValue(flattenName, entry.getValue()); + mapColumns.add(flattenName); + } + record = flattenMap(record, mapColumns); + } else if (record.getValue(column) instanceof Collection && _collectionsToUnnest.contains(column)) { + for (Object inner : (Collection) record.getValue(column)) { + if (inner instanceof Map) { Review comment: Did you add a TODO somewhere? ########## File path: pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ComplexTypeTransformer.java ########## @@ -0,0 +1,229 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.segment.local.recordtransformer; + +import com.google.common.annotations.VisibleForTesting; +import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.List; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.pinot.spi.config.table.TableConfig; +import org.apache.pinot.spi.config.table.ingestion.ComplexTypeConfig; +import org.apache.pinot.spi.data.readers.GenericRow; + + +/** + * A transformer to handle the complex types such as Map and Collection, with flattening and unnesting. + * <p> + * The map flattening rule will recursively flatten all the map types, except for those under the collection that is not marked as to unnest. + * + * For example: + * <pre> + * { + * "t1":{ + * "array":[ + * { + * "t2":{ + * "a":"v1" + * } + * } + * ] + * } + * } + * </pre> + * + * flattens to + * <pre> + * { + * "t1.array":[ + * { + * "t2.a":"v1" + * } + * ] + * } + * <pre/> + * + * <p> + * + * The unnesting rule will flatten all the collections provided, which are the paths navigating to the collections. For + * the same example above. If the the collectionToUnnest is provided as "t1.array", then the rule will unnest the + * previous output to: + * + * <pre> + * [{ + * "t1.arrayt2.a": "v1", + * }] + * * <pre/> + * + * Note the unnest rule will output a collection of generic rows under the field {@link GenericRow#MULTIPLE_RECORDS_KEY}. + * + */ +public class ComplexTypeTransformer implements RecordTransformer { + private static final CharSequence DELIMITER = "."; + private final List<String> _collectionsToUnnest; + + public ComplexTypeTransformer(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() != null + && tableConfig.getIngestionConfig().getComplexTypeConfig() != null) { + _collectionsToUnnest = + tableConfig.getIngestionConfig().getComplexTypeConfig().getUnnestConfig() != null ? tableConfig + .getIngestionConfig().getComplexTypeConfig().getUnnestConfig() : new ArrayList<>(); + } else { + _collectionsToUnnest = new ArrayList<>(); + } + } + + @VisibleForTesting + public ComplexTypeTransformer(List<String> unnestCollections) { + _collectionsToUnnest = new ArrayList<>(unnestCollections); + Collections.sort(_collectionsToUnnest); + } + + public static boolean isComplexTypeHandlingEnabled(TableConfig tableConfig) { + if (tableConfig.getIngestionConfig() == null + || tableConfig.getIngestionConfig().getComplexTypeConfig() == null + || tableConfig.getIngestionConfig().getComplexTypeConfig().getMode() == null) { + return false; + } + return tableConfig.getIngestionConfig().getComplexTypeConfig().getMode() + != ComplexTypeConfig.Mode.NONE; + } + + @Nullable + @Override + public GenericRow transform(GenericRow record) { + flattenMap(record, new HashSet<>(record.getFieldToValueMap().keySet())); + for (String collection : _collectionsToUnnest) { + unnestCollection(record, collection); + } + return record; + } + + private GenericRow unnestCollection(GenericRow record, String column) { + if (record.getValue(GenericRow.MULTIPLE_RECORDS_KEY) == null) { Review comment: Cache the value to avoid the redundant `getValue()` on line 117 ########## File path: pinot-spi/src/main/java/org/apache/pinot/spi/data/readers/GenericRow.java ########## @@ -120,6 +126,49 @@ public boolean hasNullValues() { return !_nullValueFields.isEmpty(); } + /** + * @return a deep copy of the generic row + */ + public GenericRow copy() { Review comment: I think we don't need deep copy because we never modify the value in-place. Using shallow copy can significantly reduce the overhead -- 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