[Feature](lambda) Support some map lambda functions - #66968
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
5c42734 to
2130e12
Compare
|
/review |
There was a problem hiding this comment.
Automated review status: complete and converged after three rounds. Requesting changes for three P1 correctness issues.
Findings
- The FE materialization rule moves sensitive Map inputs from Join candidate-pair scope into a child-row Project, changing outer-join error behavior and volatile evaluation frequency.
- The same rule moves computed Maps out of selector-controlled IF/CASE branches, defeating supported short-circuit execution.
- The BE eight-batch direct path bypasses the variable-width lambda safety ceiling and can turn a bounded query into a multi-gigabyte ColumnString overflow.
Checkpoint conclusions
- Goal and scope: the advertised Map lambda wrappers, constructors, tuple syntax, recursive NULL-type handling, and FE-to-BE lowerings are present. The scalar wrappers and registrations are localized; the 786-line late materialization rule and the unrelated ArrayMap batching expansion are the principal risk surfaces.
- Planner semantics and parallel paths: Project, OneRowRelation, Filter, Having, normalized Aggregate, Generate, Join metadata, nested lambda ExprIds, marker translation, and second-pass stability were reviewed. Outside the two inline evaluation-domain failures, no additional owner or parallel path defect survived.
- Types, nullability, compatibility, and physical symbols: lambda arity, Map-key legality, nullable Maps/predicates, nested NULL_TYPE merging, last-win semantics, and the names
map_from_arrays,%map_from_arrays_unique%,map_filter, andmap_from_entriesalign across FE and BE. No serialized format or existing function signature changes. - Runtime correctness and ownership: constant/nullable wrappers, offsets, selected rows, invalid entries, duplicate keys, COW detachment, and Status propagation were checked. Those paths remain sound outside the accepted lambda budget issue.
- Performance and memory: the direct-path multiplier is a correctness failure as well as an allocation regression; no other distinct performance defect survived review.
- Concurrency, lifecycle, configuration, persistence, transactions, writes, and observability: no new applicable mechanism is introduced.
- Tests: the changed unit and regression sources broadly cover ordinary, null, constant, duplicate, empty, nested, aggregate, Join, Generate, and selected-row behavior, but omit the three accepted boundaries. Per the automated-review contract, I did not run builds or tests; current style, license, formatting, and secret checks pass, while build/test jobs are skipped.
- User focus: no additional focus was supplied.
No pre-existing live review thread or comment duplicated these findings.
|
run buildall |
TPC-H: Total hot run time: 17554 ms |
TPC-DS: Total hot run time: 84739 ms |
ClickBench: Total hot run time: 14.67 s |
FE UT Coverage ReportIncrement line coverage |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
1872ace to
9651dc4
Compare
|
/review |
There was a problem hiding this comment.
Automated review status: complete and converged after three rounds. Requesting changes for one new P1 correctness issue.
Finding
- Native-table access-path pruning can lose Map values used only inside a nested Lambda and substitute default/null values; one inline comment is attached.
- The existing P1 variable-width Lambda budget thread remains applicable and was not duplicated.
Critical checkpoint conclusions
- Goal and proof: the PR adds Map Lambda functions, Map constructors/filtering, tuple Lambda syntax, and FE-to-BE lowering. Registrations and ordinary semantics align, and unit/regression sources cover null, empty, duplicate, constant, computed, nondeterministic, nested, aggregate, Join, and Generate cases; the accepted native stored-column boundary is not covered.
- Scope and focus: the implementation is cohesive for the advertised feature and reuses ArrayMap/MapEntries/Map primitives. No additional user focus was supplied.
- Concurrency: execution uses query-local expression/column state; no new shared mutable state, locks, lock ordering, or thread-safety mechanism is introduced.
- Lifecycle/static initialization: Lambda frames use scoped guards and ColumnMap sharing uses COW; no non-intuitive release path, circular ownership, or cross-TU static initialization dependency was found.
- Configuration: no configuration item or dynamic-update path is added.
- Compatibility: FE wrapper names, arities, return/nullability types, and internal physical symbols match BE registration. No storage format, persisted metadata, or protocol variable changes are introduced; the new function symbols are additive.
- Parallel paths: public Map functions, internal unique/filtered constructors, legacy two-argument
map_filter, constant folding, computed inputs, nested Lambdas, and native/external scan paths were checked. The native scan divergence is the attached finding. - Conditional checks: const broadcasting, top-level null Maps/arrays, hidden null-row payloads, mismatched offsets, nullable predicates, and last-win deduplication checks have explicit failure/skip behavior and targeted tests. No separate conditional-check defect survived.
- Test coverage: BE unit tests and FE/regression tests are broad, including negative cases, but need a stored native-table test where the outer Map Lambda retains the key and reads the value only inside an inner Lambda.
- Test results: the committed
.outresults match the asserted ordinary semantics. Per the review-run contract, I did not run builds or tests. - Observability: these are scalar analysis/execution paths; existing AnalysisException/Status failures are sufficient, and no new log or metric is required.
- Transactions/persistence: no transaction, EditLog, master-failover, or persistent-state path is changed.
- Data writes/crashes: the feature is query-time only and does not modify stored data. COW detachment and Status propagation are sound outside the accepted read-pruning correctness issue.
- FE-BE variables: no new session/protocol variable is transmitted. FE and BE independently computed types/names were checked and agree.
- Performance: zero-copy/COW paths are reasonable, but the existing live byte-budget P1 remains unresolved; no distinct additional CPU/memory issue survived duplicate fencing.
- Other issues: the scalar-subquery tuple-parser candidate was rejected because the semantic predicate only gates, rather than forces, its alternative. No other unresolved candidate remains.
|
run buildall |
TPC-H: Total hot run time: 17112 ms |
TPC-DS: Total hot run time: 82668 ms |
ClickBench: Total hot run time: 14.61 s |
FE UT Coverage ReportIncrement line coverage |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 16923 ms |
FE UT Coverage ReportIncrement line coverage |
TPC-DS: Total hot run time: 81934 ms |
ClickBench: Total hot run time: 14.62 s |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
1 similar comment
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
| args+=errorCapturingIdentifier (COMMA args+=errorCapturingIdentifier)+ | ||
| RIGHT_PAREN | ||
| ARROW body=booleanExpression | ||
| ARROW ({isTupleLambdaBody()}? |
There was a problem hiding this comment.
这个 {isTupleLambdaBody()}? 做过 benchmark 吗?确定可以提升性能吗?
There was a problem hiding this comment.
只是为了支持语法糖 (k, v) -> (k * 3, v + 1), 估计还可能存在回退
There was a problem hiding this comment.
没有这个也可以支持的吧,这里只不过在不满足的情况下,去掉了这个分支
There was a problem hiding this comment.
原先是不支持这个语法糖的
Doris> select array_map((k, v) -> (k + 1, v + 1), [1], [1]);
、ERROR 1105 (HY000): errCode = 2, detailMessage =
missing ')' at ','(line 1, pos 33)
Doris> select array_map((k, v) -> struct(k + 1, v + 1), [1], [1]);
+-----------------------------------------------------+
| array_map((k, v) -> struct(k + 1, v + 1), [1], [1]) |
+-----------------------------------------------------+
| [{"col1":2, "col2":2}] |
+-----------------------------------------------------+
测了下,性能稳定劣化, 可能没支持的必要?
| SQL 场景 | 无 predicate | 当前 predicate | 变化 |
|---|---|---|---|
| 单个 tuple lambda | 23.749 μs/op | 25.153 μs/op | 慢 5.91% |
| 32 个 tuple lambda | 679.133 μs/op | 697.665 μs/op | 慢 2.73% |
| 单个括号普通表达式 | 20.621 μs/op | 20.734 μs/op | 慢 0.55% |
| 32 个括号普通表达式 | 572.181 μs/op | 575.088 μs/op | 慢 0.51% |
| 32 个普通表达式 | 336.354 μs/op | 338.058 μs/op | 慢 0.51% |
| private static final Set<String> MAP_ENTRY_LAMBDA_FUNCTIONS = ImmutableSet.of( | ||
| "map_all", | ||
| "map_apply", | ||
| "map_exists", | ||
| "map_filter", | ||
| "transform_keys", | ||
| "transform_values"); |
There was a problem hiding this comment.
这个list,包括下面的 bindMapLambdaFunction 可以重构以更适合扩展,参考:#67284
Issue Number: None Related PR: apache#66968 Problem Summary: Higher-order function analysis selected map handling and array comparator behavior with hardcoded function-name checks. Move lambda parameter binding contracts into builtin function registration and use the resolved function builder metadata to analyze array and map lambdas through one flow. None - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.rules.analysis.FunctionRegistryTest,org.apache.doris.nereids.trees.expressions.functions.scalar.MapLambdaFunctionsTest,org.apache.doris.nereids.rules.analysis.CheckExpressionLegalityTest,org.apache.doris.nereids.trees.expressions.functions.scalar.ArrayFirstLastTest - mvn checkstyle:check -pl fe-core - Behavior changed: No - Does this need documentation: No
Issue Number: None Related PR: apache#66968 Problem Summary: Higher-order function analysis selected map handling and array comparator behavior with hardcoded function-name checks. Move lambda parameter binding contracts into builtin function registration and use the resolved function builder metadata to analyze array and map lambdas through one flow. None - Test: Unit Test - ./run-fe-ut.sh --run org.apache.doris.nereids.rules.analysis.FunctionRegistryTest,org.apache.doris.nereids.trees.expressions.functions.scalar.MapLambdaFunctionsTest,org.apache.doris.nereids.rules.analysis.CheckExpressionLegalityTest,org.apache.doris.nereids.trees.expressions.functions.scalar.ArrayFirstLastTest - mvn checkstyle:check -pl fe-core - Behavior changed: No - Does this need documentation: No
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 16843 ms |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-DS: Total hot run time: 81668 ms |
ClickBench: Total hot run time: 14.64 s |
| scalar(ArraySortBy.class, "array_sortby"), | ||
| scalar(ArraySplit.class, "array_split"), | ||
| lambdaScalar(ArraySort.class, LambdaBindingSpecs.ARRAY_COMPARATOR, "array_sort"), | ||
| lambdaScalar(ArraySortBy.class, LambdaBindingSpecs.ARRAY_ZIP, "array_sortby"), |
There was a problem hiding this comment.
把所有lambdaScalar 放到一起吧,按字母序排序。
FE Regression Coverage ReportIncrement line coverage |
|
run buildall |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
What problem does this PR solve?
Related PR: #67045, #67047
Doc: apache/doris-website#4085
Problem Summary:
This PR adds MAP lambda support under the Nereids planner.
Supported higher-order functions:
map_filter((k, v) -> predicate, map)map_exists((k, v) -> predicate, map)map_all((k, v) -> predicate, map)map_apply((k, v) -> struct(new_key, new_value), map)transform_keys((k, v) -> new_key, map)transform_values((k, v) -> new_value, map)Example:
Implementation
Use
map_entries(m)to expand the map parameter and reuse the originalarray lambdafor execution.Internal helper function
This PR adds two internal Map construction functions used by the rewritten expressions:
transform_valuemap_from_entries. Used bymap_filter