Copilot commented on code in PR #18875: URL: https://github.com/apache/pinot/pull/18875#discussion_r3693455862
########## pinot-query-runtime/src/main/java/org/apache/pinot/query/runtime/operator/groupby/OneUuidKeyGroupIdGenerator.java: ########## @@ -0,0 +1,83 @@ +/** + * 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.query.runtime.operator.groupby; + +import it.unimi.dsi.fastutil.objects.Object2IntMap; +import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; +import it.unimi.dsi.fastutil.objects.ObjectIterator; +import java.util.Iterator; +import java.util.function.ToIntFunction; +import org.apache.pinot.spi.utils.UuidKey; + + +/** + * Group-id generator for a single UUID group-by key in the multi-stage engine. Normalizes every incoming key + * ({@code byte[]}, {@code ByteArray}, {@code String} or {@code UuidKey}) to {@link UuidKey} — two primitive longs — + * so map probing avoids byte-array hashing/equality on the hot path. Not thread-safe; each instance is owned by a + * single operator thread, matching the other {@link GroupIdGenerator} implementations. + */ +public class OneUuidKeyGroupIdGenerator implements GroupIdGenerator { + private final Object2IntOpenHashMap<Object> _groupIdMap; + private final int _numGroupsLimit; + private final ToIntFunction<Object> _groupIdGenerator; + + public OneUuidKeyGroupIdGenerator(int numGroupsLimit, int initialCapacity) { + _groupIdMap = new Object2IntOpenHashMap<>(initialCapacity); + _groupIdMap.defaultReturnValue(INVALID_ID); + _numGroupsLimit = numGroupsLimit; + _groupIdGenerator = ignored -> _groupIdMap.size(); + } + + @Override + public int getGroupId(Object key) { + Object normalizedKey = key != null ? UuidKey.fromObject(key) : null; + if (_groupIdMap.size() < _numGroupsLimit) { + return _groupIdMap.computeIfAbsent(normalizedKey, _groupIdGenerator); + } else { + return _groupIdMap.getInt(normalizedKey); + } Review Comment: Using Object2IntOpenHashMap#computeIfAbsent here likely goes through the java.util.Map default method (boxing Integer) instead of fastutil's primitive specialization. On the group-by hot path this adds avoidable allocations/boxing. Use computeIntIfAbsent to keep the map primitive. ########## pinot-core/src/main/java/org/apache/pinot/core/segment/processing/partitioner/TableConfigPartitioner.java: ########## @@ -53,6 +67,10 @@ public String getPartitionFromColumns(Object[] columnValues) { throw new IllegalArgumentException( "TableConfigPartitioner expects exactly 1 column value, got " + columnValues.length); } - return String.valueOf(_partitionFunction.getPartition(FieldSpec.getStringValue(columnValues[0]))); + return String.valueOf(_partitionFunction.getPartition(toPartitionString(columnValues[0]))); + } + + private String toPartitionString(Object value) { + return _dataType != null ? _dataType.toString(value) : FieldSpec.getStringValue(value); } Review Comment: toPartitionString() now routes through FieldSpec.DataType#toString(Object) when _dataType is set, but DataType#toString expects raw byte[] for BYTES/UUID. GenericRow / default-null plumbing can surface ByteArray wrappers for these types, which would now cause a ClassCastException. Unwrap ByteArray to byte[] before calling _dataType.toString so UUID renders canonically and BYTES stays hex without crashing. -- 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]
