From bc729e8afe76157042ab0418b3ec9ff3b6939564 Mon Sep 17 00:00:00 2001 From: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> Date: Sat, 12 Oct 2024 14:30:27 +0800 Subject: [PATCH 1/3] fix array_map's error result when inputs are constant Signed-off-by: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> --- be/src/exprs/array_map_expr.cpp | 30 ++++++++++++++--------- test/sql/test_array_fn/R/test_array_map_2 | 6 ++--- 2 files changed, 21 insertions(+), 15 deletions(-) diff --git a/be/src/exprs/array_map_expr.cpp b/be/src/exprs/array_map_expr.cpp index 1f2f9d3d2da53..074d362ca9f87 100644 --- a/be/src/exprs/array_map_expr.cpp +++ b/be/src/exprs/array_map_expr.cpp @@ -99,8 +99,12 @@ StatusOr ArrayMapExpr::evaluate_lambda_expr(ExprContext* context, Chu auto array_column = down_cast(data_column.get()); auto elements_column = array_column->elements_column(); UInt32Column::Ptr offsets_column = array_column->offsets_column(); - if constexpr (!all_const_input) { - if (input_elements[i]->is_constant()) { + + if (input_elements[i]->is_constant()) { + if (!all_const_input || !capture_slot_ids.empty()) { + // if not all input columns are constant, + // or all input columns are constant but lambda expr depends on other capture columns(e.g. array_map(x->x+k,[1,2,3])), + // we should unpack the const column before evaluation size_t elements_num = array_column->get_element_size(0); elements_column = elements_column->clone(); offsets_column = UInt32Column::create(); @@ -114,13 +118,13 @@ StatusOr ArrayMapExpr::evaluate_lambda_expr(ExprContext* context, Chu offset += elements_num; offsets_column->append(offset); } - } else { - if (result_null_column != nullptr) { - data_column->empty_null_in_complex_column(result_null_column->get_data(), - array_column->offsets().get_data()); - } - elements_column = down_cast(data_column.get())->elements_column(); } + } else { + if (result_null_column != nulltpr) { + data_column->empty_null_in_complex_column(result_null_column->get_data(), + array_column->offsets().get_data()); + } + elements_column = down_cast(data_column.get())->elements_column(); } if (aligned_offsets == nullptr) { @@ -173,8 +177,9 @@ StatusOr ArrayMapExpr::evaluate_lambda_expr(ExprContext* context, Chu column = tmp_col->replicate(aligned_offsets->get_data()); column = ColumnHelper::align_return_type(column, type().children[0], column->size(), true); } else { - // if all input arguments are const, - if constexpr (all_const_input) { + // if all input arguments are constant and lambda expr doesn't rely on other capture columns, + // we can evaluate it based on const column + if (all_const_input && capture_slot_ids.empty()) { ASSIGN_OR_RETURN(auto tmp_col, context->evaluate(_children[0], cur_chunk.get())); tmp_col->check_or_die(); // if result is a const column, we should unpack it first and make it to be the elements column of array column @@ -200,8 +205,9 @@ StatusOr ArrayMapExpr::evaluate_lambda_expr(ExprContext* context, Chu DCHECK(column != nullptr); column = ColumnHelper::cast_to_nullable_column(column); - if constexpr (all_const_input) { - // if all input arguments are const, we can return a const column + if (all_const_input && capture_slot_ids.empty()) { + // if all input arguments are const and lambdaexpr doesn't depend on other capture columns, + // we can return a const column auto data_column = FunctionHelper::get_data_column_of_const(column); aligned_offsets = UInt32Column::create(); diff --git a/test/sql/test_array_fn/R/test_array_map_2 b/test/sql/test_array_fn/R/test_array_map_2 index ed5bacc3ea490..db23d0e0695b7 100644 --- a/test/sql/test_array_fn/R/test_array_map_2 +++ b/test/sql/test_array_fn/R/test_array_map_2 @@ -130,9 +130,9 @@ None select array_map((x,y)->k, [1,2],[2,3]) from t order by k; -- result: [1,1] -[1,1] -[1,1] -[1,1] +[2,2] +[3,3] +[4,4] -- !result select array_map((x,y,z)->x+y+z, [1,2],[2,3],[3,4]) from t; -- result: From e17332d107a97ca526fc243a45df2601b83c8e58 Mon Sep 17 00:00:00 2001 From: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> Date: Sat, 12 Oct 2024 14:48:56 +0800 Subject: [PATCH 2/3] fix format Signed-off-by: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> --- be/src/exprs/array_map_expr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be/src/exprs/array_map_expr.cpp b/be/src/exprs/array_map_expr.cpp index 074d362ca9f87..335d6a287ce5e 100644 --- a/be/src/exprs/array_map_expr.cpp +++ b/be/src/exprs/array_map_expr.cpp @@ -122,7 +122,7 @@ StatusOr ArrayMapExpr::evaluate_lambda_expr(ExprContext* context, Chu } else { if (result_null_column != nulltpr) { data_column->empty_null_in_complex_column(result_null_column->get_data(), - array_column->offsets().get_data()); + array_column->offsets().get_data()); } elements_column = down_cast(data_column.get())->elements_column(); } From 39df0a087f553faa63ae48dfafedef7fd2ae5ee3 Mon Sep 17 00:00:00 2001 From: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> Date: Sat, 12 Oct 2024 19:22:40 +0800 Subject: [PATCH 3/3] fix Signed-off-by: silverbullet233 <3675229+silverbullet233@users.noreply.github.com> --- be/src/exprs/array_map_expr.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/be/src/exprs/array_map_expr.cpp b/be/src/exprs/array_map_expr.cpp index 335d6a287ce5e..90420f60f248a 100644 --- a/be/src/exprs/array_map_expr.cpp +++ b/be/src/exprs/array_map_expr.cpp @@ -120,7 +120,7 @@ StatusOr ArrayMapExpr::evaluate_lambda_expr(ExprContext* context, Chu } } } else { - if (result_null_column != nulltpr) { + if (result_null_column != nullptr) { data_column->empty_null_in_complex_column(result_null_column->get_data(), array_column->offsets().get_data()); }