xiangfu0 commented on code in PR #18875:
URL: https://github.com/apache/pinot/pull/18875#discussion_r3800427602
##########
pinot-common/src/main/java/org/apache/pinot/common/utils/DataSchema.java:
##########
@@ -303,6 +324,16 @@ public RelDataType toType(RelDataTypeFactory typeFactory) {
return typeFactory.createSqlType(SqlTypeName.MAP);
}
},
+ // NOTE: UUID is placed before OBJECT and the array types, shifting their
ordinals by +1 relative to any build that
+ // does not contain this enum constant. This is safe because DataSchema
serialization uses enum names (not ordinals)
+ // via ColumnDataType.name() / ColumnDataType.valueOf(). If ordinal-based
serialization is ever added for
+ // ColumnDataType, UUID must be moved to the end of the enum (as was done
for FieldSpec.DataType.UUID).
+ UUID(BYTES, null) {
Review Comment:
Removed from this PR during the rebase onto merged UUID parts 1-6. UUID 7
now contains no DataSchema or broker/server wire-type change; the current diff
is limited to the opt-in partition function and focused tests. Resolving this
outdated thread as out of scope here.
##########
pinot-common/src/main/proto/expressions.proto:
##########
@@ -44,6 +44,12 @@ enum ColumnDataType {
UNKNOWN = 19;
MAP = 20;
BIG_DECIMAL_ARRAY = 21;
+ // Rolling-upgrade limitation for UUID columns: in a mixed-version
multi-stage query, an older broker/server that
+ // does not know UUID = 22 / UUID_ARRAY = 23 will fail planning with
UnknownEnumValueException when receiving a plan
+ // that includes a UUID literal. Avoid issuing UUID queries until all
brokers and servers are upgraded. See the
+ // matching note on DataSchema.toBytes and
ProtoExpressionToRexExpression#convertColumnDataType.
+ UUID = 22;
Review Comment:
Removed from this PR during the rebase onto merged UUID parts 1-6. UUID 7
introduces no proto enum or planner wire-format change; the current diff is
limited to the opt-in partition function and focused tests. Resolving this
outdated thread as out of scope here.
##########
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:
Removed. UUID 6 merged without this specialization, and UUID grouping
remains on the generic stored-BYTES and ByteArray path. There is no group-by
hot-path code in the current UUID 7 diff.
##########
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:
Removed the schema-threading and TableConfigPartitioner changes from UUID 7.
The current implementation no longer routes UUID or BYTES values through
DataType.toString here; representation normalization is confined to
UuidPartitionFunction.
--
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]
