krishan1390 commented on code in PR #16845: URL: https://github.com/apache/pinot/pull/16845#discussion_r2403683358
########## pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/stats/NoDictColumnStatisticsCollector.java: ########## @@ -0,0 +1,232 @@ +/** + * 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.segment.creator.impl.stats; + +import com.clearspring.analytics.stream.cardinality.HyperLogLogPlus; +import java.math.BigDecimal; +import java.nio.charset.StandardCharsets; +import org.apache.commons.lang3.NotImplementedException; +import org.apache.pinot.segment.spi.creator.StatsCollectorConfig; +import org.apache.pinot.spi.utils.BigDecimalUtils; +import org.apache.pinot.spi.utils.ByteArray; +import org.apache.pinot.spi.utils.CommonConstants; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + + +/** + * Column statistics collector for no-dictionary columns that avoids storing unique values and thus reduces memory + * Behavior: + * - getUniqueValuesSet() throws NotImplementedException + * - getCardinality() returns approximate cardinality using HLL++ + * - Doesn't handle cases where values are of different types (e.g. int and long). This is expected. + * Individual type collectors (e.g. IntColumnPreIndexStatsCollector) also don't handle this case. + * At this point in the Pinot process, the type consistency of a key should already be enforced. + * So if such a case is encountered, it will be raised as an exception during collect() + * Doesn't handle MAP data type as MapColumnPreIndexStatsCollector is optimized for no-dictionary collection + */ +@SuppressWarnings({"rawtypes"}) +public class NoDictColumnStatisticsCollector extends AbstractColumnStatisticsCollector { + private static final Logger LOGGER = LoggerFactory.getLogger(NoDictColumnStatisticsCollector.class); + private Comparable _minValue; + private Comparable _maxValue; + private int _minLength = Integer.MAX_VALUE; + private int _maxLength = -1; // default return value is -1 + private int _maxRowLength = -1; // default return value is -1 + private boolean _sealed = false; + // HLL Plus generally returns approximate cardinality >= actual cardinality which is desired + private final HyperLogLogPlus _hllPlus; + + public NoDictColumnStatisticsCollector(String column, StatsCollectorConfig statsCollectorConfig) { + super(column, statsCollectorConfig); + // Use default p and sp; can be made configurable via StatsCollectorConfig later if needed + _hllPlus = new HyperLogLogPlus( + CommonConstants.Helix.DEFAULT_HYPERLOGLOG_PLUS_P, + CommonConstants.Helix.DEFAULT_HYPERLOGLOG_PLUS_SP); + LOGGER.info("Initialized NoDictColumnStatisticsCollector for column: {}", column); + } + + @Override + public void collect(Object entry) { + assert !_sealed; + if (entry instanceof Object[]) { + Object[] values = (Object[]) entry; + int rowLength = 0; + for (Object value : values) { + if (value instanceof BigDecimal) { + // BigDecimalColumnPreIndexStatsCollector doesn't support multi-value + throw new UnsupportedOperationException(); + } + updateMinMax(value); + updateHllPlus(value); + int len = getValueLength(value); + _minLength = Math.min(_minLength, len); + _maxLength = Math.max(_maxLength, len); + rowLength += len; + } + _maxNumberOfMultiValues = Math.max(_maxNumberOfMultiValues, values.length); + _maxRowLength = Math.max(_maxRowLength, rowLength); + updateTotalNumberOfEntries(values); + } else if (entry instanceof int[] || entry instanceof long[] + || entry instanceof float[] || entry instanceof double[]) { + // Native multi-value types don't require length calculation because they're not variable-length + int length; + if (entry instanceof int[]) { + int[] values = (int[]) entry; + for (int value : values) { + updateMinMax(value); + updateHllPlus(value); + } + length = values.length; + } else if (entry instanceof long[]) { + long[] values = (long[]) entry; + for (long value : values) { + updateMinMax(value); + updateHllPlus(value); + } + length = values.length; + } else if (entry instanceof float[]) { + float[] values = (float[]) entry; + for (float value : values) { + updateMinMax(value); + updateHllPlus(value); + } + length = values.length; + } else { + double[] values = (double[]) entry; + for (double value : values) { + updateMinMax(value); + updateHllPlus(value); + } + length = values.length; + } + _maxNumberOfMultiValues = Math.max(_maxNumberOfMultiValues, length); + updateTotalNumberOfEntries(length); + } else { + Comparable value = toComparable(entry); + addressSorted(value); + updateMinMax(entry); + updateHllPlus(entry); + int len = getValueLength(entry); + _minLength = Math.min(_minLength, len); + _maxLength = Math.max(_maxLength, len); + if (isPartitionEnabled()) { + updatePartition(value.toString()); + } + _maxRowLength = Math.max(_maxRowLength, len); + _totalNumberOfEntries++; + } + } + + private void updateMinMax(Object value) { + Comparable comp = toComparable(value); + if (_minValue == null || comp.compareTo(_minValue) < 0) { + _minValue = comp; + } + if (_maxValue == null || comp.compareTo(_maxValue) > 0) { + _maxValue = comp; + } + } + + private Comparable toComparable(Object value) { + if (value instanceof byte[]) { + return new ByteArray((byte[]) value); + } + if (value instanceof Comparable) { + return (Comparable) value; + } + throw new IllegalStateException("Unsupported value type " + value.getClass()); + } + + private int getValueLength(Object value) { + if (value instanceof byte[]) { + return ((byte[]) value).length; + } + if (value instanceof CharSequence) { + return ((CharSequence) value).toString().getBytes(StandardCharsets.UTF_8).length; + } + if (value instanceof BigDecimal) { + return BigDecimalUtils.byteSize((BigDecimal) value); + } + if (value instanceof Number) { + return 8; // fixed-width approximation as it's not actually required for numeric fields which are of fixed length + } + throw new IllegalStateException("Unsupported value type " + value.getClass()); + } + + @Override + public Object getMinValue() { + if (_sealed) { + return _minValue; + } + throw new IllegalStateException("you must seal the collector first before asking for min value"); + } + + @Override + public Object getMaxValue() { + if (_sealed) { + return _maxValue; + } + throw new IllegalStateException("you must seal the collector first before asking for max value"); + } + + @Override + public Object getUniqueValuesSet() { + throw new NotImplementedException("getUniqueValuesSet is not supported in NoDictColumnStatisticsCollector"); Review Comment: ColumnIndexCreationInfo.getDistinctValueCount returns Constants.UNKNOWN_CARDINALITY if getUniqueValuesSet is null. I didn't want to change the behaviour in ColumnIndexCreationInfo.getDistinctValueCount because it might break something -- 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]
