hubgeter commented on code in PR #69682:
URL: https://github.com/apache/doris/pull/69682#discussion_r4413134609
##########
be/src/core/data_type/data_type.h:
##########
@@ -155,6 +155,12 @@ class IDataType : private boost::noncopyable {
virtual char* serialize(const IColumn& column, char* buf, int
be_exec_version) const = 0;
virtual const char* deserialize(const char* buf, MutableColumnPtr* column,
int be_exec_version) const = 0;
+ // Variable-length types can override this overload to validate embedded
lengths
+ // against end. Fixed-size types can use the default forwarding.
+ virtual const char* deserialize(const char* buf, const char* end,
MutableColumnPtr* column,
+ int be_exec_version) const {
Review Comment:
Fixed in c250b82d25793256a924e7adfa74f978129e5d3c. Array, Map and Struct now
implement the end-aware overload in both wire layouts and forward the bound to
every child. Array/Map validate the serialized row header and offset byte count
before allocation/copy. The checked const-header helper is shared with
HLL/Nullable; the trusted three-argument interfaces retain their existing
layout.
Added eight layout/wrapper cases for Array, Map, Struct and a mixed
Nullable/Array/Map/Struct chain. They cover every truncated serialized prefix,
oversized offset counts, the remote PBlock with a FULL HLL length but only its
tag, valid PBlock decoding, and const 0/1/512-row round trips through both
interfaces with serialized-byte comparison.
All changed production objects compiled in DEBUG, and focused
safety/complexity clang-tidy passed. The focused runtime UT build is still
running; this reply does not claim those tests have executed.
##########
be/src/exec/exchange/vdata_stream_recvr.cpp:
##########
@@ -68,18 +68,18 @@ VDataStreamRecvr::SenderQueue::~SenderQueue() {
}
Status VDataStreamRecvr::SenderQueue::get_batch(Block* block, bool* eos) {
-#ifndef NDEBUG
- if (!_is_cancelled && _block_queue.empty() && _num_remaining_senders > 0) {
- throw doris::Exception(ErrorCode::INTERNAL_ERROR,
- "_is_cancelled: {}, _block_queue_empty: {}, "
- "_num_remaining_senders: {},
_debug_string_info: {}",
- _is_cancelled, _block_queue.empty(),
_num_remaining_senders,
- _debug_string_info());
- }
-#endif
BlockItem block_item;
{
INJECT_MOCK_SLEEP(std::lock_guard<std::mutex> l(_lock));
+#ifndef NDEBUG
+ if (!_is_cancelled && _block_queue.empty() && _num_remaining_senders >
0) {
+ throw doris::Exception(ErrorCode::INTERNAL_ERROR,
+ "_is_cancelled: {}, _block_queue_empty: {},
"
+ "_num_remaining_senders: {},
_debug_string_info: {}",
+ _is_cancelled, _block_queue.empty(),
_num_remaining_senders,
+ _debug_string_info());
+ }
+#endif
//check and get block_item from data_queue
if (_is_cancelled) {
Review Comment:
Confirmed this accounting issue predates this backport, but it is directly
relevant to the queue-cleanup contract. Fixed in
c250b82d25793256a924e7adfa74f978129e5d3c: close sums only the BlockItems still
in the queue under its lock and releases their receiver/profile and queue
accounting before clearing them. An item already popped by get_batch is
excluded and remains the responsibility of its Defer, so concurrent teardown
cannot release that item twice.
Added a mixed local/remote buffered-close case checking both MemTrackers,
current profile bytes, callback count and repeated close. The success/error
callback-concurrency cases also check empty accounting after cancel/close. The
changed production object compiled in DEBUG and focused safety/complexity
clang-tidy passed. Runtime UT is still building; executed results will be
reported separately.
--
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]
