Gabriel39 commented on code in PR #68297:
URL: https://github.com/apache/doris/pull/68297#discussion_r4060820541
##########
be/src/exec/common/hash_table/hash_key_type.h:
##########
@@ -102,6 +102,13 @@ inline HashKeyType get_hash_key_type_fixed(const
std::vector<DataTypePtr>& data_
}
inline HashKeyType get_hash_key_type(const std::vector<DataTypePtr>&
data_types) {
+ // Reject binary before the multi-key serialization fallback can enable
joins or grouping.
+ for (const auto& type : data_types) {
+ if (type->get_primitive_type() == TYPE_VARBINARY) {
Review Comment:
The complex-type serialized hash paths already existed before this PR. A
recursive FE/BE capability fence is broader than fixing newly introduced
correctness/stability defects, and is not included.
##########
be/src/exprs/aggregate/aggregate_function_min_max_impl.h:
##########
@@ -140,6 +140,10 @@ AggregateFunctionPtr
create_aggregate_function_single_value(const String& name,
return creator_without_type::create_unary_arguments<
AggregateFunctionsSingleValue<Data<SingleValueDataComplexType>>>(
argument_types, result_is_nullable, attr);
+ case PrimitiveType::TYPE_VARBINARY:
Review Comment:
The existing complex aggregate dispatch and recursive comparison paths were
not introduced by this PR. Extending the restriction recursively across sibling
factories is outside this follow-up.
##########
be/src/exprs/function/in.h:
##########
@@ -105,6 +105,10 @@ class FunctionIn : public IFunction {
if (scope == FunctionContext::THREAD_LOCAL) {
return Status::OK();
}
+ // Binary IO must not route IN through the shared string/storage
predicate implementation.
+ if (context->get_arg_type(0)->get_primitive_type() == TYPE_VARBINARY) {
Review Comment:
The collection_in routing and missing nested VARBINARY CRC implementation
predate this PR. Adding a recursive FE capability inventory is outside this
follow-up.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/agg/CollectSet.java:
##########
@@ -108,4 +109,14 @@ public List<FunctionSignature> getSignatures() {
public Expression resultForEmptyInput() {
return new ArrayLiteral(new ArrayList<>(), this.getDataType());
}
+
+ @Override
+ public void checkLegalityBeforeTypeCoercion() {
+ // The BE set kernel cannot hash raw VARBINARY; reject it before
implicit casts change its type.
+ for (Expression argument : getArguments()) {
+ if (argument.getDataType().isVarBinaryType()) {
Review Comment:
The nested aggregate dispatch limitations and sibling aggregate omissions
predate this PR. This follow-up does not expand the recursive FE validation
inventory or add aggregate support.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayFunctionUtils.java:
##########
@@ -0,0 +1,42 @@
+// 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.doris.nereids.trees.expressions.functions.scalar;
+
+import org.apache.doris.nereids.exceptions.AnalysisException;
+import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.types.ArrayType;
+import org.apache.doris.nereids.types.DataType;
+
+/** Argument validation shared by array functions. */
+final class ArrayFunctionUtils {
+ private ArrayFunctionUtils() {
+ }
+
+ static void checkNoVarBinaryArguments(ScalarFunction function) {
+ // Inspect original arguments before coercion can hide unsupported
binary comparison/hash inputs.
Review Comment:
The cited sibling collection/map computations and dispatcher limitations
already existed. A recursive policy across all collection functions would
broaden the PR and is not included.
--
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]
