github-actions[bot] commented on code in PR #64624:
URL: https://github.com/apache/doris/pull/64624#discussion_r3529751164
##########
be/src/exprs/vectorized_agg_fn.cpp:
##########
@@ -381,14 +413,26 @@ Status AggFnEvaluator::_calc_argument_columns(Block*
block) {
SCOPED_TIMER(_expr_timer);
_agg_columns.resize(_input_exprs_ctxs.size());
std::vector<int> column_ids(_input_exprs_ctxs.size());
+ std::vector<int> materialize_column_ids;
+ materialize_column_ids.reserve(_input_exprs_ctxs.size());
for (int i = 0; i < _input_exprs_ctxs.size(); ++i) {
+ if (_const_argument_idx[i]) {
+ continue;
+ }
int column_id = -1;
RETURN_IF_ERROR(_input_exprs_ctxs[i]->execute(block, &column_id));
column_ids[i] = column_id;
+ materialize_column_ids.push_back(column_id);
+ }
+ if (!materialize_column_ids.empty()) {
+ materialize_block_inplace(*block, materialize_column_ids.data(),
+ materialize_column_ids.data() +
materialize_column_ids.size());
}
- materialize_block_inplace(*block, column_ids.data(),
- column_ids.data() + _input_exprs_ctxs.size());
for (int i = 0; i < _input_exprs_ctxs.size(); ++i) {
+ if (_const_argument_idx[i]) {
+ _agg_columns[i] = nullptr;
+ continue;
Review Comment:
`_calc_argument_columns()` now removes the cached const argument columns,
but the rest of the aggregate input-column contract still expects every slot to
be a real `IColumn`. For example `exponential_moving_average(5.0, v, t)` marks
argument 0 const, this branch stores `_agg_columns[0] = nullptr`, and then
`execute_batch_add()` immediately calls
`_function->check_input_columns_type(_agg_columns.data())`. The EMA checker
still calls `check_argument_column_type<ColumnFloat64>(columns[0])`, which
dereferences that null pointer before `add()` can use the cached `_half_decay`.
The same pattern affects the percentile/reservoir/AI_AGG checkers, and the
`_state` wrapper now pushes `nullptr` for cached args before calling
`_agg_function->check_input_columns_type()` and
`streaming_agg_serialize_to_column()`. Please keep the evaluated const column
available through type checking/serialization, or make those APIs explicitly
const-argument-aware instead of passing `nullptr` through the existin
g contract.
--
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]
