You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "BiteTheDDDDt (via GitHub)" <gi...@apache.org> on 2026/01/26 04:01:21 UTC
[PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
BiteTheDDDDt opened a new pull request, #60228:
URL: https://github.com/apache/doris/pull/60228
### What problem does this PR solve?
Issue Number: close #xxx
Related PR: #xxx
Problem Summary:
### Release note
None
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [ ] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Behavior changed:
- [ ] No.
- [ ] Yes. <!-- Explain the behavior change -->
- Does this need documentation?
- [ ] No.
- [ ] Yes. <!-- Add document PR link here. eg: https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR should merge into -->
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3834531510
# FE UT Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_42fde50eff0a2989321a0756dc2553b95007d005/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_42fde50eff0a2989321a0756dc2553b95007d005/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3797876310
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3835554529
# BE Regression && UT Coverage Report
Increment line coverage `82.27% (478/581)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_42fde50eff0a2989321a0756dc2553b95007d005_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_42fde50eff0a2989321a0756dc2553b95007d005_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.28% (26473/36125) |
| Line Coverage | 56.36% (280848/498305) |
| Region Coverage | 54.15% (235609/435073) |
| Branch Coverage | 55.71% (101198/181665) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3852676377
# FE Regression Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_ecc79e2d42299b0d064f23e68bf5058790049b7d_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_ecc79e2d42299b0d064f23e68bf5058790049b7d_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3859580933
# FE Regression Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_fbffb0daa908fe0cdc5b77d83f251754df435161_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_fbffb0daa908fe0cdc5b77d83f251754df435161_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3799560490
# Cloud UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/3d0bde617b14b39101086bda84583125bc7b3b1c_3d0bde617b14b39101086bda84583125bc7b3b1c_cloud/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/3d0bde617b14b39101086bda84583125bc7b3b1c_3d0bde617b14b39101086bda84583125bc7b3b1c_cloud/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 79.32% (1787/2253) |
| Line Coverage | 64.76% (31823/49139) |
| Region Coverage | 65.51% (15871/24228) |
| Branch Coverage | 56.06% (8433/15042) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3834091788
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3834186188
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3841309739
# FE UT Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_3e448c8d98058754c9039b00bba45dd8b0262010/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_3e448c8d98058754c9039b00bba45dd8b0262010/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3797811415
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3798654212
# FE Regression Coverage Report
Increment line coverage ` 0.00% (0/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_e2fb3cac1f12e4ab462936a025d6f896782cff0d_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_e2fb3cac1f12e4ab462936a025d6f896782cff0d_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3857461756
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3799427973
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #60228:
URL: https://github.com/apache/doris/pull/60228#discussion_r2726300456
##########
gensrc/thrift/PaloInternalService.thrift:
##########
@@ -423,7 +423,7 @@ struct TQueryOptions {
// runtime profiling to choose the most efficient algorithm for the data pattern
183: optional bool enable_use_hybrid_sort = false;
184: optional i32 cte_max_recursion_depth;
-
+ 185: optional bool enable_aggregate_function_null_v2 = false;
Review Comment:
The PR description is missing critical information. It should include:
1. What problem does aggregate_function_null_v2 solve?
2. What are the differences between V1 and V2?
3. Why is this change needed?
4. What testing has been done, especially for multi-argument aggregate functions?
The Issue Number field shows "close #xxx" which is a placeholder and should reference an actual issue.
##########
be/src/pipeline/exec/aggregation_source_operator.cpp:
##########
@@ -500,9 +500,6 @@ Status AggLocalState::merge_with_serialized_key_helper(vectorized::Block* block)
for (int i = 0; i < Base::_shared_state->aggregate_evaluators.size(); ++i) {
auto col_id = Base::_shared_state->probe_expr_ctxs.size() + i;
auto column = block->get_by_position(col_id).column;
- if (column->is_nullable()) {
- column = ((vectorized::ColumnNullable*)column.get())->get_nested_column_ptr();
- }
Review Comment:
The removal of nullable column unwrapping logic is incompatible with the incomplete V2 implementation. When enable_aggregate_function_null_v2 is true and multi-argument aggregate functions are used, they will receive nullable columns but the V1 variadic wrapper (still being used due to the bug in helpers.h) expects non-nullable columns. This change should not have been made until the V2 variadic implementation is complete.
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -1461,6 +1462,9 @@ public enum IgnoreSplitType {
needForward = true)
private boolean enableParallelScan = true;
+ @VariableMgr.VarAttr(name = ENABLE_AGGREGATE_FUNCTION_NULL_V2, fuzzy = true, needForward = true)
+ private boolean enableAggregateFunctionNullV2 = true;
Review Comment:
This PR lacks test coverage for the new aggregate_function_null_v2 feature. Given the critical bugs identified (incomplete V2 implementation for multi-argument functions and removed unwrapping logic), comprehensive tests are essential. Tests should cover:
1. Single-argument aggregate functions with nullable inputs (e.g., SUM, AVG, COUNT with nullable columns)
2. Multi-argument aggregate functions with nullable inputs (e.g., COVAR, map_agg, min_max_by)
3. Window functions with nullable inputs
4. Both V1 and V2 implementations to ensure compatibility
5. Serialization and deserialization with nullable columns
The existing test infrastructure in be/test/vec/aggregate_functions/ should be used to add tests for this feature.
##########
be/src/vec/aggregate_functions/helpers.h:
##########
@@ -85,6 +87,11 @@ struct creator_without_type {
using NullableT = std::conditional_t<multi_arguments, AggregateFunctionNullVariadicInline<T, f>,
AggregateFunctionNullUnaryInline<T, f>>;
+ template <bool multi_arguments, bool f, typename T>
+ using NullableV2T =
+ std::conditional_t<multi_arguments, AggregateFunctionNullVariadicInline<T, f>,
Review Comment:
The V2 implementation is incomplete. The NullableV2T template alias uses AggregateFunctionNullVariadicInline (the old V1 implementation) for multi-argument functions instead of a V2 variadic implementation. This means when enable_aggregate_function_null_v2 is true, multi-argument aggregate functions will still use the V1 variadic wrapper, which expects columns to be unwrapped (non-nullable), but the operator code changes removed the unwrapping logic. This will cause multi-argument aggregate functions to fail or behave incorrectly.
A AggregateFunctionNullVariadicInlineV2 class needs to be implemented in aggregate_function_null_v2.h, similar to AggregateFunctionNullUnaryInlineV2.
```suggestion
std::conditional_t<multi_arguments, AggregateFunctionNullVariadicInlineV2<T, f>,
```
##########
fe/fe-core/src/main/java/org/apache/doris/qe/SessionVariable.java:
##########
@@ -1461,6 +1462,9 @@ public enum IgnoreSplitType {
needForward = true)
private boolean enableParallelScan = true;
+ @VariableMgr.VarAttr(name = ENABLE_AGGREGATE_FUNCTION_NULL_V2, fuzzy = true, needForward = true)
+ private boolean enableAggregateFunctionNullV2 = true;
Review Comment:
There is a default value mismatch. The Thrift definition sets enable_aggregate_function_null_v2 to false by default (line 426 in PaloInternalService.thrift), but the SessionVariable sets it to true by default. This inconsistency can lead to unexpected behavior where the FE thinks the feature is enabled but BE queries might not have it set. The defaults should be aligned.
```suggestion
private boolean enableAggregateFunctionNullV2 = false;
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3799783409
# FE UT Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_3d0bde617b14b39101086bda84583125bc7b3b1c/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_3d0bde617b14b39101086bda84583125bc7b3b1c/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3808747314
# BE Regression && UT Coverage Report
Increment line coverage `83.73% (453/541)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.15% (26190/35804) |
| Line Coverage | 56.24% (279148/496336) |
| Region Coverage | 54.05% (232816/430705) |
| Branch Coverage | 55.65% (100200/180066) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3805261105
# FE Regression Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3858252719
# Cloud UT Coverage Report
Increment line coverage ` ` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/fbffb0daa908fe0cdc5b77d83f251754df435161_fbffb0daa908fe0cdc5b77d83f251754df435161_cloud/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/fbffb0daa908fe0cdc5b77d83f251754df435161_fbffb0daa908fe0cdc5b77d83f251754df435161_cloud/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 79.29% (1792/2260) |
| Line Coverage | 64.78% (31845/49158) |
| Region Coverage | 65.43% (15887/24280) |
| Branch Coverage | 55.99% (8442/15078) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3852624171
# BE Regression && UT Coverage Report
Increment line coverage `82.38% (477/579)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_ecc79e2d42299b0d064f23e68bf5058790049b7d_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_ecc79e2d42299b0d064f23e68bf5058790049b7d_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 71.67% (25907/36146) |
| Line Coverage | 54.34% (270816/498410) |
| Region Coverage | 51.90% (225829/435090) |
| Branch Coverage | 53.29% (96804/181667) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3859496141
# BE Regression && UT Coverage Report
Increment line coverage `82.21% (476/579)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_fbffb0daa908fe0cdc5b77d83f251754df435161_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_fbffb0daa908fe0cdc5b77d83f251754df435161_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.38% (26548/36177) |
| Line Coverage | 56.49% (281930/499074) |
| Region Coverage | 54.12% (235877/435860) |
| Branch Coverage | 55.80% (101506/181902) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3851209290
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "Thearas (via GitHub)" <gi...@apache.org>.
Thearas commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3797811312
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR).
Please clearly describe your PR:
1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3801191355
# FE Regression Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_3d0bde617b14b39101086bda84583125bc7b3b1c_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_3d0bde617b14b39101086bda84583125bc7b3b1c_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3835611711
# FE Regression Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_42fde50eff0a2989321a0756dc2553b95007d005_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_42fde50eff0a2989321a0756dc2553b95007d005_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3895501067
PR approved by at least one committer and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt merged PR #60228:
URL: https://github.com/apache/doris/pull/60228
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3851201084
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3840939131
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3842621512
# FE Regression Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_3e448c8d98058754c9039b00bba45dd8b0262010_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_3e448c8d98058754c9039b00bba45dd8b0262010_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3851428752
# FE UT Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_ecc79e2d42299b0d064f23e68bf5058790049b7d/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_ecc79e2d42299b0d064f23e68bf5058790049b7d/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3869166683
# BE Regression && UT Coverage Report
Increment line coverage `82.21% (476/579)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_fbffb0daa908fe0cdc5b77d83f251754df435161_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_fbffb0daa908fe0cdc5b77d83f251754df435161_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.39% (26550/36177) |
| Line Coverage | 56.50% (281960/499074) |
| Region Coverage | 54.13% (235922/435860) |
| Branch Coverage | 55.81% (101515/181902) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3869191357
# FE Regression Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_fbffb0daa908fe0cdc5b77d83f251754df435161_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_fbffb0daa908fe0cdc5b77d83f251754df435161_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3852274115
# BE UT Coverage Report
Increment line coverage `8.98% (52/579)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/ecc79e2d42299b0d064f23e68bf5058790049b7d_ecc79e2d42299b0d064f23e68bf5058790049b7d/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/ecc79e2d42299b0d064f23e68bf5058790049b7d_ecc79e2d42299b0d064f23e68bf5058790049b7d/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 52.52% (19370/36883) |
| Line Coverage | 36.01% (179908/499598) |
| Region Coverage | 32.41% (139569/430672) |
| Branch Coverage | 33.39% (60410/180935) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3797879652
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3804108470
# FE UT Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387/fe_increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387/fe_report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3805199987
# BE Regression && UT Coverage Report
Increment line coverage `83.73% (453/541)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387_merge/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387_merge/report/index.html)
| Category | Coverage |
|-------------------|--------------------|
| Function Coverage | 73.15% (26190/35804) |
| Line Coverage | 56.24% (279139/496336) |
| Region Coverage | 54.06% (232835/430705) |
| Branch Coverage | 55.64% (100191/180066) |
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #60228:
URL: https://github.com/apache/doris/pull/60228#discussion_r2730816015
##########
gensrc/thrift/PaloInternalService.thrift:
##########
@@ -424,8 +424,8 @@ struct TQueryOptions {
183: optional bool enable_use_hybrid_sort = false;
184: optional i32 cte_max_recursion_depth;
-
185: optional bool enable_parquet_file_page_cache = true;
+ 186: optional bool enable_aggregate_function_null_v2 = false;
Review Comment:
There is a mismatch between the default values for enable_aggregate_function_null_v2. The Java SessionVariable defaults to true (line 1469 in SessionVariable.java), but the Thrift definition defaults to false (line 428 in PaloInternalService.thrift). This inconsistency could lead to different behavior depending on whether the value is explicitly set or uses the default. The defaults should be aligned.
```suggestion
186: optional bool enable_aggregate_function_null_v2 = true;
```
##########
be/src/vec/aggregate_functions/aggregate_function_null_v2.h:
##########
@@ -0,0 +1,598 @@
+// 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.
+// This file is copied from
+// https://github.com/ClickHouse/ClickHouse/blob/master/src/AggregateFunctions/AggregateFunctionNull.h
+// and modified by Doris
+
+#pragma once
+
+#include <glog/logging.h>
+
+#include <memory>
+
+#include "common/logging.h"
+#include "vec/aggregate_functions/aggregate_function.h"
+#include "vec/aggregate_functions/aggregate_function_distinct.h"
+#include "vec/columns/column.h"
+#include "vec/columns/column_nullable.h"
+#include "vec/common/assert_cast.h"
+#include "vec/common/string_buffer.hpp"
+#include "vec/core/types.h"
+#include "vec/data_types/data_type_nullable.h"
+
+namespace doris::vectorized {
+#include "common/compile_check_begin.h"
+
+template <typename NestFunction, bool result_is_nullable, typename Derived>
+class AggregateFunctionNullBaseInlineV2 : public IAggregateFunctionHelper<Derived> {
+protected:
+ std::unique_ptr<NestFunction> nested_function;
+ size_t prefix_size;
+ bool is_window_function = false;
+
+ AggregateDataPtr nested_place(AggregateDataPtr __restrict place) const noexcept {
+ return place + prefix_size;
+ }
+
+ ConstAggregateDataPtr nested_place(ConstAggregateDataPtr __restrict place) const noexcept {
+ return place + prefix_size;
+ }
+
+ static void init(AggregateDataPtr __restrict place, bool is_window_function) noexcept {
+ init_flag(place);
+ init_null_count(place, is_window_function);
+ }
+
+ static void init_flag(AggregateDataPtr __restrict place) noexcept {
+ if constexpr (result_is_nullable) {
+ place[0] = false;
+ }
+ }
+
+ static void set_flag(AggregateDataPtr __restrict place) noexcept {
+ if constexpr (result_is_nullable) {
+ place[0] = true;
+ }
+ }
+
+ static bool get_flag(ConstAggregateDataPtr __restrict place) noexcept {
+ return result_is_nullable ? place[0] : true;
+ }
+
+ static void init_null_count(AggregateDataPtr __restrict place,
+ bool is_window_function) noexcept {
+ if (is_window_function && result_is_nullable) {
+ unaligned_store<int32_t>(place + 1, 0);
+ }
+ }
+
+ static void update_null_count(AggregateDataPtr __restrict place, bool incremental,
+ bool is_window_function) noexcept {
+ if (is_window_function && result_is_nullable) {
+ auto null_count = unaligned_load<int32_t>(place + 1);
+ incremental ? null_count++ : null_count--;
+ unaligned_store<int32_t>(place + 1, null_count);
+ }
+ }
+
+ static int32_t get_null_count(ConstAggregateDataPtr __restrict place,
+ bool is_window_function) noexcept {
+ int32_t num = 0;
+ if (is_window_function && result_is_nullable) {
+ num = unaligned_load<int32_t>(place + 1);
+ }
+ return num;
+ }
+
+public:
+ AggregateFunctionNullBaseInlineV2(IAggregateFunction* nested_function_,
+ const DataTypes& arguments, bool is_window_function_)
+ : IAggregateFunctionHelper<Derived>(arguments),
+ nested_function {assert_cast<NestFunction*>(nested_function_)},
+ is_window_function(is_window_function_) {
+ DCHECK(nested_function_ != nullptr);
+ if constexpr (result_is_nullable) {
+ if (this->is_window_function) {
+ // flag|---null_count----|-------padding-------|--nested_data----|
+ size_t nested_align = nested_function->align_of_data();
+ prefix_size = 1 + sizeof(int32_t);
+ if (prefix_size % nested_align != 0) {
+ prefix_size += (nested_align - (prefix_size % nested_align));
+ }
+ } else {
+ prefix_size = nested_function->align_of_data();
+ }
+ } else {
+ prefix_size = 0;
+ }
+ }
+
+ MutableColumnPtr create_serialize_column() const override {
+ if constexpr (result_is_nullable) {
+ return ColumnNullable::create(nested_function->create_serialize_column(),
+ ColumnUInt8::create());
+ }
+ return nested_function->create_serialize_column();
+ }
+
+ DataTypePtr get_serialized_type() const override {
+ if constexpr (result_is_nullable) {
+ return make_nullable(nested_function->get_serialized_type());
+ }
+ return nested_function->get_serialized_type();
+ }
+
+ void set_query_context(QueryContext* ctx) override {
+ return nested_function->set_query_context(ctx);
+ }
+
+ bool is_blockable() const override { return nested_function->is_blockable(); }
+
+ void set_version(const int version_) override {
+ IAggregateFunctionHelper<Derived>::set_version(version_);
+ nested_function->set_version(version_);
+ }
+
+ String get_name() const override { return "NullableV2(" + nested_function->get_name() + ")"; }
+
+ DataTypePtr get_return_type() const override {
+ return result_is_nullable ? make_nullable(nested_function->get_return_type())
+ : nested_function->get_return_type();
+ }
+
+ void create(AggregateDataPtr __restrict place) const override {
+ init(place, this->is_window_function);
+ nested_function->create(nested_place(place));
+ }
+
+ void destroy(AggregateDataPtr __restrict place) const noexcept override {
+ nested_function->destroy(nested_place(place));
+ }
+ void reset(AggregateDataPtr place) const override {
+ init(place, this->is_window_function);
+ nested_function->reset(nested_place(place));
+ }
+
+ bool is_trivial() const override { return false; }
+
+ size_t size_of_data() const override { return prefix_size + nested_function->size_of_data(); }
+
+ size_t align_of_data() const override {
+ if (this->is_window_function && result_is_nullable) {
+ return std::max(nested_function->align_of_data(), alignof(int32_t));
+ } else {
+ return nested_function->align_of_data();
+ }
+ }
+
+ void merge(AggregateDataPtr __restrict place, ConstAggregateDataPtr rhs,
+ Arena& arena) const override {
+ if (get_flag(rhs)) {
+ set_flag(place);
+ nested_function->merge(nested_place(place), nested_place(rhs), arena);
+ }
+ }
+
+ void serialize(ConstAggregateDataPtr __restrict place, BufferWritable& buf) const override {
+ bool flag = get_flag(place);
+ if constexpr (result_is_nullable) {
+ buf.write_binary(flag);
+ }
+ if (flag) {
+ nested_function->serialize(nested_place(place), buf);
+ }
+ }
+
+ void deserialize(AggregateDataPtr __restrict place, BufferReadable& buf,
+ Arena& arena) const override {
+ bool flag = true;
+ if constexpr (result_is_nullable) {
+ buf.read_binary(flag);
+ }
+ if (flag) {
+ set_flag(place);
+ nested_function->deserialize(nested_place(place), buf, arena);
+ }
+ }
+
+ void serialize_to_column(const std::vector<AggregateDataPtr>& places, size_t offset,
+ MutableColumnPtr& dst, const size_t num_rows) const override {
+ if constexpr (result_is_nullable) {
+ auto& nullable_col = assert_cast<ColumnNullable&>(*dst);
+ auto& nested_col = nullable_col.get_nested_column();
+ auto& null_map = nullable_col.get_null_map_data();
+ MutableColumnPtr nested_col_ptr = nested_col.assume_mutable();
+
+ null_map.resize(num_rows);
+ uint8_t* __restrict null_map_data = null_map.data();
+ for (size_t i = 0; i < num_rows; ++i) {
+ null_map_data[i] = !get_flag(places[i] + offset);
+ }
+ nested_function->serialize_to_column(places, offset + prefix_size, nested_col_ptr,
+ num_rows);
+ } else {
+ nested_function->serialize_to_column(places, offset, dst, num_rows);
+ }
+ }
+
+ void streaming_agg_serialize_to_column(const IColumn** columns, MutableColumnPtr& dst,
+ const size_t num_rows, Arena& arena) const override {
+ const auto* src_nullable_col = assert_cast<const ColumnNullable*>(columns[0]);
+ const auto* __restrict src_null_map_data = src_nullable_col->get_null_map_data().data();
+
+ size_t nested_size = nested_function->size_of_data();
+ std::vector<AggregateDataPtr> nested_places(num_rows);
+ std::vector<char> places_data(num_rows * nested_size);
+ for (size_t i = 0; i < num_rows; ++i) {
+ nested_places[i] = places_data.data() + i * nested_size;
+ }
+
+ if (!nested_function->is_trivial()) {
+ for (int i = 0; i < num_rows; ++i) {
+ try {
+ nested_function->create(nested_places[i]);
+ } catch (...) {
+ for (int j = 0; j < i; ++j) {
+ nested_function->destroy(nested_places[j]);
+ }
+ throw;
+ }
+ }
+ }
+ Defer destroy_places = {[&]() {
+ if (!nested_function->is_trivial()) {
+ for (int i = 0; i < num_rows; ++i) {
+ nested_function->destroy(nested_places[i]);
+ }
+ }
+ }};
+ const IColumn* src_nested_column =
+ src_nullable_col->get_nested_column().assume_mutable().get();
+ if (src_nullable_col->has_null()) {
+ for (size_t i = 0; i < num_rows; ++i) {
+ if (!src_null_map_data[i]) {
+ nested_function->add(nested_places[i], &src_nested_column, i, arena);
+ }
+ }
+ } else {
+ nested_function->add_batch(num_rows, nested_places.data(), 0, &src_nested_column, arena,
+ false);
+ }
+
+ if constexpr (result_is_nullable) {
+ auto& dst_nullable_col = assert_cast<ColumnNullable&>(*dst);
+ MutableColumnPtr nested_col_ptr = dst_nullable_col.get_nested_column().assume_mutable();
+ dst_nullable_col.get_null_map_column().insert_range_from(
+ src_nullable_col->get_null_map_column(), 0, num_rows);
+ nested_function->serialize_to_column(nested_places, 0, nested_col_ptr, num_rows);
+ } else {
+ nested_function->serialize_to_column(nested_places, 0, dst, num_rows);
+ }
+ }
+
+ void serialize_without_key_to_column(ConstAggregateDataPtr __restrict place,
+ IColumn& to) const override {
+ if constexpr (result_is_nullable) {
+ auto& nullable_col = assert_cast<ColumnNullable&>(to);
+ auto& nested_col = nullable_col.get_nested_column();
+ auto& null_map = nullable_col.get_null_map_data();
+
+ bool flag = get_flag(place);
+ if (flag) {
+ nested_function->serialize_without_key_to_column(nested_place(place), nested_col);
+ null_map.push_back(0);
+ } else {
+ nested_col.insert_default();
+ null_map.push_back(1);
+ }
+ } else {
+ nested_function->serialize_without_key_to_column(nested_place(place), to);
+ }
+ }
+
+ void deserialize_and_merge_vec(const AggregateDataPtr* places, size_t offset,
+ AggregateDataPtr rhs, const IColumn* column, Arena& arena,
+ const size_t num_rows) const override {
+ if constexpr (result_is_nullable) {
+ const auto& nullable_col = assert_cast<const ColumnNullable&>(*column);
+ const auto& nested_col = nullable_col.get_nested_column();
+ const auto* __restrict null_map_data = nullable_col.get_null_map_data().data();
+
+ for (size_t i = 0; i < num_rows; ++i) {
+ *(places[i] + offset) |= (!null_map_data[i]);
+ }
+ nested_function->deserialize_and_merge_vec(places, offset + prefix_size, rhs,
+ &nested_col, arena, num_rows);
+ } else {
+ this->nested_function->deserialize_and_merge_vec(places, offset, rhs, column, arena,
+ num_rows);
+ }
+ }
+
+ void deserialize_and_merge_vec_selected(const AggregateDataPtr* places, size_t offset,
+ AggregateDataPtr rhs, const IColumn* column,
+ Arena& arena, const size_t num_rows) const override {
+ if constexpr (result_is_nullable) {
+ const auto& nullable_col = assert_cast<const ColumnNullable&>(*column);
+ const auto& nested_col = nullable_col.get_nested_column();
+ const auto* __restrict null_map_data = nullable_col.get_null_map_data().data();
+
+ for (size_t i = 0; i < num_rows; ++i) {
+ *(places[i] + offset) |= (!null_map_data[i]);
+ }
+ nested_function->deserialize_and_merge_vec_selected(places, offset + prefix_size, rhs,
+ &nested_col, arena, num_rows);
+ } else {
+ this->nested_function->deserialize_and_merge_vec_selected(places, offset, rhs, column,
+ arena, num_rows);
+ }
+ }
+
+ void deserialize_and_merge_from_column_range(AggregateDataPtr __restrict place,
+ const IColumn& column, size_t begin, size_t end,
+ Arena& arena) const override {
+ DCHECK(end <= column.size() && begin <= end)
Review Comment:
The DCHECK condition allows end to equal column.size(), which would cause an out-of-bounds access in the loop at line 356 that uses `i <= end`. Since column indices are 0-based, the maximum valid index is column.size() - 1. The condition should be `DCHECK(end < column.size() && begin <= end)` instead.
```suggestion
DCHECK(end < column.size() && begin <= end)
```
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3803729398
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "hello-stephen (via GitHub)" <gi...@apache.org>.
hello-stephen commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3808775500
# FE Regression Coverage Report
Increment line coverage ` 66.67% (2/3)` :tada:
[Increment coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387_merge_fe/increment_report/index.html)
[Complete coverage report](http://coverage.selectdb-in.cc/coverage/60228_8fa34244848d566242925354b75e8b1d5b5d1387_merge_fe/report/index.html)
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3802794938
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "BiteTheDDDDt (via GitHub)" <gi...@apache.org>.
BiteTheDDDDt commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3858060913
run buildall
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [Feature](aggregation) support aggregate_function_null_v2 [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #60228:
URL: https://github.com/apache/doris/pull/60228#issuecomment-3895501251
PR approved by anyone and no changes requested.
--
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: commits-unsubscribe@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org