github-actions[bot] commented on code in PR #67822:
URL: https://github.com/apache/doris/pull/67822#discussion_r4012203688
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArraySort.java:
##########
@@ -62,13 +63,8 @@ private ArraySort(ScalarFunctionParams functionParams) {
public void checkLegalityBeforeTypeCoercion() {
DataType argType = getArgument(0).getDataType();
if (argType instanceof ArrayType) {
- // Find the innermost element type for nested arrays
DataType itemType = ((ArrayType) argType).getItemType();
- while (itemType.isArrayType()) {
- itemType = ((ArrayType) itemType).getItemType();
- }
- if (itemType.isMapType() || itemType.isStructType()
- || itemType.isVariantType() || itemType.isJsonType()) {
+ if
(!ArrayFunctionTypeChecker.isSupportedByArrayComparisonFunctions(itemType)) {
Review Comment:
[P2] Validate the bound source type for lambda array_sort
For the lambda overload, `getArgument(0)` has `LambdaType`, so this new
`ArrayType` branch never checks the bound source array. For example,
`array_sort((x, y) -> if(bitmap_count(x) < bitmap_count(y), -1,
if(bitmap_count(x) = bitmap_count(y), 0, 1)), array(to_bitmap(2),
to_bitmap(1)))` has a valid comparator and passes FE analysis;
`ExpressionTranslator` then sends the bound bitmap array to
`varray_sort_function.cpp`, where `get_data_from_type(TYPE_BITMAP)` returns
`InternalError("Unsupported type in array_sort")` before the lambda executes.
This is separate from the direct `compare_at` path covered by the existing
thread. Please validate `lambda.getLambdaArgument(0).getArrayExpression()`
against the lambda sorter's physical support (which is not identical to the
direct recursive-comparison set), or extend that dispatcher, and add a
lambda-form negative test.
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayEnumerateUniq.java:
##########
@@ -74,6 +75,17 @@ public void checkLegalityBeforeTypeCoercion() {
}
}
}
+ if (getArguments().size() != 1) {
Review Comment:
[P2] Validate the multi-array serialization path
This early return makes the new physical-support check single-argument-only.
`array_enumerate_uniq(array(to_bitmap(1), to_bitmap(1)), array(to_bitmap(2),
to_bitmap(2)))` still passes the earlier guard because `BitmapType` is a FE
`PrimitiveType`, not a complex type. The BE multi-argument branch then
unconditionally uses `MethodSerialized`; key initialization calls
`get_max_row_byte_size()` through the nullable wrapper into `ColumnBitmap`,
which inherits the default `NOT_IMPLEMENTED` method (and its row serializer is
also unimplemented), so the query fails before hashing. The existing discussion
covers only this function's separate single-argument `dispatch_switch_scalar`
path. Please validate every multi-array item against the serialized-key
capability set before returning, or implement the accepted serializers, and add
a two-array BITMAP negative test.
--
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]
