github-actions[bot] commented on code in PR #67822: URL: https://github.com/apache/doris/pull/67822#discussion_r4004844410
########## regression-test/suites/nereids_function_p0/scalar_function/array_function_invalid_argument.groovy: ########## @@ -0,0 +1,85 @@ +// 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. + +suite("array_function_invalid_argument") { + test { + sql "select array_flatten(1)" + exception "array_flatten requires an ARRAY argument, but got TINYINT" + } + + test { + sql "select array_compact(1)" + exception "array_compact requires an ARRAY argument, but got TINYINT" + } + + test { + sql "select array_compact(array(to_bitmap(1), to_bitmap(1)))" + exception "array_compact does not support type BITMAP" + } + + test { + sql "select array_compact(array(hll_hash('a'), hll_hash('a')))" + exception "array_compact does not support type HLL" + } + + test { + sql "select array_compact(array(to_quantile_state(1, 2048), to_quantile_state(1, 2048)))" + exception "array_compact does not support type QUANTILE_STATE" + } + + test { + sql "select array_union(array(to_bitmap(1)), array(to_bitmap(1)))" + exception "array_union does not support element type BITMAP" + } + + test { + sql "select array_union(array(cast('a' as varbinary)), array(cast('b' as varbinary)))" + exception "array_union does not support element type VARBINARY" + } + + test { + sql "select array_union(array(cast('12:34:56' as time(0))), array(cast('12:34:57' as time(0))))" + exception "array_union does not support element type TIME" + } + + test { + sql "select array_intersect(array(to_bitmap(1)), array(to_bitmap(1)))" + exception "array_intersect does not support element type BITMAP" + } + + test { + sql "select array_intersect(array(cast('a' as varbinary)), array(cast('b' as varbinary)))" + exception "array_intersect does not support element type VARBINARY" + } + + test { + sql "select array_intersect(array(cast('12:34:56' as time(0))), array(cast('12:34:57' as time(0))))" + exception "array_intersect does not support element type TIME" + } + + test { + sql "select array_except(array(to_bitmap(1)), array(to_bitmap(1)))" Review Comment: [P2] Complete the physical-support fence across parallel array paths This new negative case covers `array_except`, but other equality/hash paths still admit the same unsupported physical columns in FE. For example, `array_distinct(array(to_bitmap(1), to_bitmap(1)))` and single-argument `array_enumerate_uniq(...)` miss their string/`dispatch_switch_scalar` branches, while `array_position(array(to_bitmap(1)), to_bitmap(1))`, `array_contains`, `countequal`, `array_remove`, and `array_contains_all` fall through `dispatch_switch_all`; each returns a generic backend unsupported-type error. These use separate implementations from the existing `array_except` thread. Please align each FE check with its resolved BE dispatch (their support sets differ, including TIME) and add representative BITMAP/VARBINARY negative cases. ########## fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/ArrayUnion.java: ########## @@ -68,11 +69,18 @@ public ArrayUnion withChildren(List<Expression> children) { @Override public void checkLegalityBeforeTypeCoercion() { - DataType argType = getArgument(0).getDataType(); - if (argType.isArrayType() && (((ArrayType) argType).getItemType().isComplexType() - || ((ArrayType) argType).getItemType().isVariantType() - || ((ArrayType) argType).getItemType().isJsonType())) { - throw new AnalysisException("array_union does not support types: " + argType.toSql()); + for (Expression child : getArguments()) { + DataType argType = child.getDataType(); + if (argType == NullType.INSTANCE) { + continue; + } + if (!argType.isArrayType()) { + throw new AnalysisException("array_union requires ARRAY arguments, but got " + argType.toSql()); + } + DataType itemType = ((ArrayType) argType).getItemType(); + if (!itemType.canBeUsedInArraySetOperation()) { Review Comment: [P1] Preserve supported common-type coercions This check runs on each source item before indexed `AnyDataType(0)` chooses the common type. For example, `array_union(array(cast('a' as varbinary)), array('b'))` would normally resolve both inputs to `ARRAY<STRING>`: string-high-priority common-type selection permits VARBINARY-to-STRING, and the backend supports the resulting `ColumnString`. The new loop rejects the raw VARBINARY first, so a previously valid query now fails analysis; `ArrayExcept.java:71` has the same regression (as do mixed TIME/STRING operands). This is distinct from the existing thread about identical VARBINARY/TIME pairs, whose resolved physical types really are unsupported. Please validate the resolved element type, or otherwise preserve supported coercible pairs, and add positive mixed-type cases. ########## regression-test/suites/nereids_function_p0/scalar_function/array_function_invalid_argument.groovy: ########## @@ -0,0 +1,85 @@ +// 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. + +suite("array_function_invalid_argument") { + test { + sql "select array_flatten(1)" + exception "array_flatten requires an ARRAY argument, but got TINYINT" + } + + test { + sql "select array_compact(1)" + exception "array_compact requires an ARRAY argument, but got TINYINT" + } + + test { + sql "select array_compact(array(to_bitmap(1), to_bitmap(1)))" Review Comment: [P2] Extend the comparability fence to every comparator path This test covers the unsupported production comparator in `array_compact`, but metric columns still reach `compare_at` elsewhere. `array_sort` and `array_reverse_sort` admit BITMAP through their MAP/STRUCT/VARIANT/JSON denylist; `array_min`/`array_max` reject only complex types and compare subsequent metric elements; and `array_sortby([1, 2], [to_bitmap(1), to_bitmap(2)])` validates argument 0 even though the BE comparator sorts by argument 1. All reach the unavailable production metric comparator. Please apply a recursive comparability check to every comparator input and add representative negative coverage. -- 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]
