Copilot commented on code in PR #61535:
URL: https://github.com/apache/doris/pull/61535#discussion_r3037567427
##########
be/src/runtime/runtime_state.h:
##########
@@ -138,7 +138,40 @@ class RuntimeState {
const DescriptorTbl& desc_tbl() const { return *_desc_tbl; }
void set_desc_tbl(const DescriptorTbl* desc_tbl) { _desc_tbl = desc_tbl; }
+
+ MOCK_FUNCTION int block_max_rows() const {
+ return config::enable_adaptive_batch_size
+ ? std::max(_query_options.batch_size,
int(preferred_block_size_rows()))
+ : _query_options.batch_size;
Review Comment:
block_max_rows() casts preferred_block_size_rows() (size_t) to int inside
std::max. Since preferred_block_size_rows is user-controlled (session var
forwarded via thrift) and has no obvious upper-bound enforcement here, this
cast can overflow/underflow and produce an incorrect (possibly negative) row
limit. Consider clamping preferred_block_size_rows to INT_MAX (or changing
block_max_rows() to return size_t) before casting/combining with batch_size.
##########
gensrc/thrift/PaloInternalService.thrift:
##########
@@ -473,6 +473,18 @@ struct TQueryOptions {
210: optional double max_scan_mem_ratio = 0.3;
211: optional bool enable_adaptive_scan = false;
+
+ // Adaptive batch size: target output block size in bytes. 0 means disabled
(fixed row count only).
+ // Default 8MB. Sent by FE session variable preferred_block_size_bytes.
+ 212: optional i64 preferred_block_size_bytes = 8388608
+
+ // Per-column byte limit for adaptive batch size. 0 means no per-column
limit. Default 1MB.
+ 213: optional i64 preferred_max_column_in_block_size_bytes = 1048576
+
+ // Adaptive batch size: target output block row count limit. Default 65535.
+ // Sent by FE session variable preferred_block_size_rows.
+ 214: optional i64 preferred_block_size_rows = 65535
Review Comment:
The PR description mentions adding “Thrift fields 204/205 for FE→BE
propagation”, but the new adaptive batch size fields added here are 212/213/214
(204/205 are already used for spill_*). Please update the PR description (or
the field IDs if that was the intent) to avoid confusion for reviewers and
future maintainers.
--
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]
