github-actions[bot] commented on code in PR #68471:
URL: https://github.com/apache/doris/pull/68471#discussion_r4089992475
##########
be/src/exprs/lambda_function/varray_map_function.cpp:
##########
@@ -332,52 +366,104 @@ class ArrayMapFunction : public LambdaFunction {
}
private:
- bool _contains_column_id(const std::vector<int>& output_slot_ref_indexs,
int id) const {
- const auto it = std::find(output_slot_ref_indexs.begin(),
output_slot_ref_indexs.end(), id);
- return it != output_slot_ref_indexs.end();
+ struct LambdaArgumentBinding {
+ bool bind_by_name = true;
+ size_t argument_size = 0;
+ std::vector<std::string> names;
+ };
+
+ Status _prepare_lambda_argument_binding(const VExprSPtr& expr, size_t
expected_argument_size,
+ LambdaArgumentBinding&
argument_binding) const {
+ DORIS_CHECK_EQ(expr->node_type(), TExprNodeType::LAMBDA_FUNCTION_EXPR);
+ const auto* lambda_expr = assert_cast<const
VLambdaFunctionExpr*>(expr.get());
+
+ argument_binding.argument_size = 0;
+ argument_binding.names.clear();
+ argument_binding.bind_by_name = lambda_expr->has_argument_names();
+
+ if (!argument_binding.bind_by_name) {
+ if (_contains_nested_lambda_call(expr->get_child(0))) {
Review Comment:
[P1] Keep metadata-free local lambdas working during rollout
Field 42 is optional, but this makes it mandatory for every nested plan from
an old FE: the new BE rejects as soon as it sees any nested lambda, even a
local-only shape such as `array_map(x -> array_map(y -> y + 1, x), [[1,2]])`
that the base BE resolved with zero gaps. The same unconditional gate exists
for nested comparator bodies in `varray_sort_function.cpp`. During an
old-FE/new-BE rolling window these previously valid queries now fail in
prepare. Please retain a compatible legacy/capability-routed path for the
supported mixed-version window rather than gating solely on optional-field
presence, and cover a local-only old-metadata case.
##########
be/src/exprs/lambda_function/varray_sort_function.cpp:
##########
@@ -224,6 +253,146 @@ class ArraySortFunction : public LambdaFunction {
return Status::OK();
}
+private:
+ Status _set_comparator_argument_gap(const VExprSPtr& expr,
+ const std::vector<std::string>*
argument_names) const {
+ if (expr->is_column_ref()) {
+ auto* ref = static_cast<VColumnRef*>(expr.get());
+ RETURN_IF_ERROR(_validate_comparator_argument_ref(*ref,
argument_names));
+ ref->set_gap(0);
+ return Status::OK();
+ }
+
+ if (expr->is_slot_ref() || expr->is_virtual_slot_ref()) {
+ return Status::NotSupported(
+ "array_sort comparator only supports its own lambda
arguments, but found "
+ "captured slot ref '{}'",
+ expr->expr_name());
+ }
+
+ if (_is_lambda_call_with_lambda_expr(expr)) {
+ // array_sort comparator arguments live in a position-based,
comparator-local frame
+ // that is invisible to nested lambda frames. Reject unsupported
nested captures during
+ // prepare, otherwise execution would later fail with an internal
missing-column error.
+ // For example, array_sort((x, y) -> array_map(z -> z + x,
nested_arr), arr) is
+ // rejected because the inner array_map lambda captures the
comparator-local x; while
+ // array_sort((x, y) -> array_map(x -> x + 1, nested_arr), arr) is
still valid because
+ // the inner x is array_map's own argument and shadows the
comparator argument.
+
RETURN_IF_ERROR(_reject_nested_lambda_capture_of_comparator_argument(
+ assert_cast<const
VLambdaFunctionExpr*>(expr->children()[0].get()),
+ argument_names));
+ for (int i = 1; i < expr->children().size(); ++i) {
+
RETURN_IF_ERROR(_set_comparator_argument_gap(expr->children()[i],
argument_names));
+ }
+ return Status::OK();
+ }
+
+ for (const auto& child : expr->children()) {
+ RETURN_IF_ERROR(_set_comparator_argument_gap(child,
argument_names));
+ }
+ return Status::OK();
+ }
+
+ Status _reject_nested_lambda_capture_of_comparator_argument(
+ const VLambdaFunctionExpr* lambda_expr,
+ const std::vector<std::string>* comparator_argument_names) const {
+ if (!lambda_expr->has_argument_names()) {
+ return Status::InternalError(
+ "Cannot validate nested lambda capture in array_sort
comparator without lambda "
+ "metadata");
+ }
+ return
_reject_nested_lambda_capture_of_comparator_argument(lambda_expr->get_child(0),
+
comparator_argument_names,
+
lambda_expr->argument_names());
+ }
+
+ Status _reject_nested_lambda_capture_of_comparator_argument(
+ const VExprSPtr& expr, const std::vector<std::string>*
comparator_argument_names,
+ const std::vector<std::string>& in_scope_lambda_argument_names)
const {
+ // Names in in_scope_lambda_argument_names are declared by the nested
lambda scopes that
+ // enclose expr. They can legally shadow array_sort comparator
argument names, so a
+ // ColumnRef matching one of these names should be treated as a local
nested-lambda
+ // argument instead of an unsupported capture from the array_sort
comparator.
+ if (expr->is_column_ref()) {
+ if (std::ranges::find(in_scope_lambda_argument_names,
expr->expr_name()) !=
Review Comment:
[P1] Preserve the second comparator binding across inner shadowing
Nereids creates comparator `y` by cloning the `x` ColumnRef and changing
only `column_id` to 1, so both refs reach BE with `expr_name() == "x"`. If a
nested lambda declares `x`, this name-only branch accepts the captured `y` as
local; at runtime `VColumnRef` resolves by name before id/gap and reads the
inner `x` column instead, silently changing the comparator result. Please
preserve the second operand's lexical name/identity (or otherwise validate the
name/id mismatch) before treating it as shadowed, and add an end-to-end case
containing both inner `x` and captured comparator `y`.
--
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]
